Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add doc section about range query for range type #35222

Merged
merged 2 commits into from
Nov 6, 2018
Merged

Add doc section about range query for range type #35222

merged 2 commits into from
Nov 6, 2018

Conversation

gskema
Copy link
Contributor

@gskema gskema commented Nov 2, 2018

I didn't know about this mapping type until I saw it in an ES presentation. It's only described in the Mapping section. It should also be referenced in the Range Query documentation.

@cbuescher cbuescher added >docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types labels Nov 5, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@cbuescher cbuescher self-assigned this Nov 5, 2018
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I think this is a great addition! I left some wording suggestions because when I didn't understand at first. I'm not 100% convinced they are better. What do you think?

==== Querying range fields

`range` queries can be used on fields of type <<range,`range`>>, allowing to match a range specified in the query
with a range field value in the document. Parameter `relation` controls how these two ranges are matched:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "The relation parameter" here?

[horizontal]
`WITHIN`::

Matches the document when document's range field is within the range specified in the query.
Copy link
Member

Choose a reason for hiding this comment

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

What about "Matches documents who's range field is entirely within the query's range"?


`CONTAINS`::

Matches the document when document's range field contains the range specified in the query.
Copy link
Member

Choose a reason for hiding this comment

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

What about "Matches documents who's range field entirely contains the query's range."?


`INTERSECTS`::

Matches the document when document's range field intersects with the range specified in the query. This is the default value.
Copy link
Member

Choose a reason for hiding this comment

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

What about "Matches documents who's range field intersects the query's range"?

Copy link
Member

Choose a reason for hiding this comment

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

Also, could you wrap this around 80 columns? We're not particularly consistent with the docs, but we're trying.

@gskema
Copy link
Contributor Author

gskema commented Nov 5, 2018

Hi @nik9000, I included your proposed changes. I also changed "This is the default value when querying range fields." for clarification.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I like it!

@nik9000
Copy link
Member

nik9000 commented Nov 5, 2018

@gskema, I've stuck this in my "to merge" queue. I have to step out but will try to get to it "soon". Which might be in ~20 hours or so. Or, if any other maintainer wants to merge it, I'm cool with that. They just have to backport it and keep an eye on the build just in case.

@gskema
Copy link
Contributor Author

gskema commented Nov 5, 2018

@nik9000 np! This is a doc fix so there's no hurry. I just hope it's ok that this PR is not against master?

@nik9000
Copy link
Member

nik9000 commented Nov 5, 2018 via email

@nik9000 nik9000 merged commit ca0447d into elastic:6.4 Nov 6, 2018
nik9000 pushed a commit that referenced this pull request Nov 6, 2018
This makes their interaction more discoverable.
nik9000 pushed a commit that referenced this pull request Nov 6, 2018
This makes their interaction more discoverable.
@nik9000
Copy link
Member

nik9000 commented Nov 6, 2018

I've merged and forward ported. Thanks @gskema!

nik9000 pushed a commit that referenced this pull request Nov 6, 2018
This makes their interaction more discoverable.
matarrese added a commit to matarrese/elasticsearch that referenced this pull request Nov 6, 2018
…-agg

* master: (528 commits)
  Register Azure max_retries setting (elastic#35286)
  add version 6.4.4
  [Docs] Add painless context details for bucket_script (elastic#35142)
  Upgrade jline to 3.8.2 (elastic#35288)
  SQL: new SQL CLI logo (elastic#35261)
  Logger: Merge ESLoggerFactory into Loggers (elastic#35146)
  Docs: Add section about range query for range type (elastic#35222)
  [ILM] change remove-policy-from-index http method from DELETE to POST (elastic#35268)
  [CCR] Forgot missing return statement,
  SQL: Fix null handling for AND and OR in SELECT (elastic#35277)
  [TEST] Mute ChangePolicyForIndexIT#testChangePolicyForIndex
  Serialize ignore_throttled also to 6.6 after backport
  Check for java 11 in buildSrc (elastic#35260)
  [TEST] increase await timeout in RemoteClusterConnectionTests
  Add missing up-to-date configuration (elastic#35255)
  Adapt Lucene BWC version
  SQL: Introduce Coalesce function (elastic#35253)
  Upgrade to lucene-8.0.0-snapshot-31d7dfe6b1 (elastic#35224)
  Fix failing ICU tests (elastic#35207)
  Prevent throttled indices to be searched through wildcards by default (elastic#34354)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants