Skip to content

Filter unsupported relation for RangeQueryBuilder#26620

Merged
jpountz merged 1 commit intoelastic:masterfrom
liketic:reject-disjoint
Sep 15, 2017
Merged

Filter unsupported relation for RangeQueryBuilder#26620
jpountz merged 1 commit intoelastic:masterfrom
liketic:reject-disjoint

Conversation

@liketic
Copy link
Copy Markdown

@liketic liketic commented Sep 13, 2017

Closes #26575
@jpountz Please help to review. Happy to make further changes. Thanks.

@elasticmachine
Copy link
Copy Markdown
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Copy Markdown
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment. It looks good to me in general, thanks for writing a test!

@liketic
Copy link
Copy Markdown
Author

liketic commented Sep 13, 2017

@jpountz Thanks! However I didn't find your comment... 😃

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit wrong to me to make ShapeRelation aware of how it is used in range queries. I'd rather put this logic in RangeQueryBuilder.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense! Very thanks!

@jpountz
Copy link
Copy Markdown
Contributor

jpountz commented Sep 13, 2017

I had forgotten to click on the button. :)

@jpountz
Copy link
Copy Markdown
Contributor

jpountz commented Sep 13, 2017

@elasticmachine please test it

@liketic
Copy link
Copy Markdown
Author

liketic commented Sep 14, 2017

@jpountz Please let me know if you plan to merge this so I can resolve the conflicts and squash commits. Thanks very much! 😄

@jpountz
Copy link
Copy Markdown
Contributor

jpountz commented Sep 14, 2017

CI check was successful so I'll merge it today.

@liketic
Copy link
Copy Markdown
Author

liketic commented Sep 14, 2017

@jpountz rebase done. Thanks!

@jpountz jpountz merged commit 0f2a116 into elastic:master Sep 15, 2017
@jpountz jpountz added :Search/Search Search-related issues that do not fall into other categories >bug v6.1.0 v7.0.0 labels Sep 15, 2017
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 16, 2017
* master:
  fix testSniffNodes to use the new error message
  Add check for invalid index in WildcardExpressionResolver (elastic#26409)
  Docs: Use single-node discovery.type for dev example
  Filter unsupported relation for range query builder (elastic#26620)
  Fix kuromoji default stoptags (elastic#26600)
  [Docs] Add description for missing fields in Reindex/Update/Delete By Query (elastic#26618)
  [Docs] Update ingest.asciidoc (elastic#26599)
  Better message text for ResponseException
  [DOCS] Remove edit link from ML node
  enable bwc testing
  fix StartRecoveryRequestTests.testSerialization
  Add bad_request to the rest-api-spec catch params (elastic#26539)
  Introduce a History UUID as a requirement for ops based recovery  (elastic#26577)
  Add missing catch arguments to the rest api spec (elastic#26536)
@liketic liketic deleted the reject-disjoint branch October 26, 2017 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search/Search Search-related issues that do not fall into other categories v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants