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

[DOCS] Rewrite terms_set query #43060

Merged
merged 5 commits into from
Jun 28, 2019
Merged

[DOCS] Rewrite terms_set query #43060

merged 5 commits into from
Jun 28, 2019

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Jun 10, 2019

Changes

  • Rewrites description of terms_set query
  • Adds example request section
  • Adds parameters sections
  • Adds notes section

This is part of #40977, an effort to standardize documentation for query types.

Before

Before image Before image

After

After image Before image

@jrodewig jrodewig added >docs General docs changes :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.2.0 v7.0.2 v7.1.2 labels Jun 10, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Thanks again @jrodewig, I didn't know this query very well and it was a pleasure reading the rewrite. Left one suggestions regarding "boost" again, also I wasn't sure about real-life use cases for this type of query so I did a bit of digging when/why this was added and found #26915. There are two more or less intuitive examples where this pretty specialized query might be useful in #26915 (comment). I don't know how far you want to take details in these reference docs but maybe there is a way of including some of these thoughts succinctly? Just an idea, maybe that makes the docs to complicated so I leave it up to you.


An example that always limits the number of required terms to match to never
become larger than the number of terms specified:
`boost`::
Copy link
Member

Choose a reason for hiding this comment

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

Like in a previous PR, maybe we could mention the "boost" parameter on all queries for completeness but link to a common paragraph for the detailed discussion.

@jrodewig
Copy link
Contributor Author

Thanks for the review and praise @cbuescher.

I've updated the example and included the listed use cases with a33ee6e. Thanks for pointing out #26915. I feel like the examples are much more practical now.

I also removed the boost parameter documentation for now with 75e12a4.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Awesome, LGTM

@jrodewig jrodewig merged commit b490eab into elastic:master Jun 28, 2019
@jrodewig jrodewig deleted the terms-set-query-rewrite branch June 28, 2019 16:56
jrodewig added a commit that referenced this pull request Jun 28, 2019
jrodewig added a commit that referenced this pull request Jun 28, 2019
jrodewig added a commit that referenced this pull request Jun 28, 2019
jrodewig added a commit that referenced this pull request Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search/Search Search-related issues that do not fall into other categories v7.0.2 v7.1.2 v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants