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 term query docs for new format #41498

Merged
merged 5 commits into from
May 6, 2019
Merged

[DOCS] Rewrite term query docs for new format #41498

merged 5 commits into from
May 6, 2019

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Apr 24, 2019

Changes

  • Simplifies description of term query
  • Replaces aside with warning to not use for text field types
  • Removes boost example
  • Adds parameters sections

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

Any and all feedback welcome!

Things to note for possible discussion

  • I intentionally chose to use a more verbose request body (including a value parameter) as it made parameter documentation a bit more clear.
  • I intentionally removed the boost example. While I think documenting boost is important, it felt out of place here. I made a note about this in [DOCS] Standardize query documentation #40977.

Before / After

Before changes

screencapture-elastic-co-guide-en-elasticsearch-reference-current-query-dsl-term-query-html-2019-04-24-12_05_41

After changes

screencapture-localhost-8000-guide-query-dsl-term-query-html-2019-05-01-18_46_53

@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.1 labels Apr 24, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search


A `term` query can also match against <<range, range data types>>.

.Why doesn't the `term` query match my document?
Copy link
Contributor

Choose a reason for hiding this comment

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

Whilst I'm not wedded to having this explanation in this page of the documentation I think it would be useful to make sure we explain the differece between searching with analyzed or not analyzed queries somewhere since it gives some understanding into how search works and how to avoid some pitfalls. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback.

I think the warning provides enough information for users looking to get started quickly, but it doesn't explain why you should avoid using term-level queries for analyzed fields. The example in the aside is great for that.

I think that content would fit better in a concept-focused page like "Search structured data" or "Search full-text." Those don't exist yet, but I'll work on creating them and add it to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds great, thanks

@jrodewig jrodewig added the WIP label Apr 26, 2019
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.

I like the cleanup and the succinctness of the rewrite in general. I'd love to see (or keep) some longer examples like the ones removed here either in later sections in the docs (maybe linked from here), or on the bottom of the page though. Nothing to block this PR, but I'd like to hear your and other opinions on this.

<3> The `full_text` inverted index will contain the terms: [`quick`, `foxes`].
<4> The `exact_value` inverted index will contain the exact term: [`Quick Foxes!`].

Now, compare the results for the `term` query and the `match` query:
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity: are there follow up plans to move these more detailed examples somewhere else? While I like the idea of having succinct, standardized docs for each query, I find these kind of examples quite useful and better to understand than merely the minimal snippet that remains. Not saying this shouldn't go away, I'm just curious what the plan is for examples like this in this rewriting effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @cbuescher.

For this particular set of examples, I think a concept-focused page like "Search structured data" or "Search full-text" would be a better fit. Those pages don't exist yet, but I'll work on creating them and add it to this PR.

For other queries, I think we can include additional sections for detailed examples below parameter documentation. A good example of this would be the missing query section for the exists query.

Let me know if you feel differently. I'm still new so there could be context I'm missing.

@colings86
Copy link
Contributor

@jrodewig IT looks like something has gone wrong with merging here. Let me know if you need help correcting it

* Simplifies description of `term` query
* Replaces aside with warning to not use for `text` field types
* Removes detailed information about boost
* Adds parameters sections
@jrodewig
Copy link
Contributor Author

Yep. Sorry about that @colings86. Looks like my rebase went wrong. Should be fixed up now.

@colings86
Copy link
Contributor

Awesome, thanks for the quick fix @jrodewig

@jrodewig
Copy link
Contributor Author

jrodewig commented May 1, 2019

Thanks again for your feedback @colings86 and @cbuescher.

With ed44215, I changed directions and re-added a revised version of the "term vs. match query" example under a new "Notes" section.

I initially proposed to include this example in a new "Search structured data" or "Search full text" page. However, after beginning work, I realized my plan had a few issues:

  • We'll still need to include usage advice for specific queries. The new "Notes" section creates a pattern for that.
  • The information I originally planned to include in the "Search structured data" or "Search full-text" pages was already covered by the existing Full text queries and Term level queries pages.
  • After revising it, I realized this example is more relevant for the "term" query than most other term level queries.

Please let me know what you think of the new changes. Thanks again.

@colings86
Copy link
Contributor

@jrodewig I like this new approach as it still provides the important information for understanding what the query is doing but keeps it from getting in the way of the details on how to specify the query

@jrodewig jrodewig merged commit 8541dd8 into elastic:master May 6, 2019
@jrodewig jrodewig deleted the term-query-rewrite branch May 6, 2019 14:36
jrodewig added a commit that referenced this pull request May 6, 2019
* [DOCS] Restructure `term` query docs.
jrodewig added a commit that referenced this pull request May 6, 2019
* [DOCS] Restructure `term` query docs.
@jrodewig jrodewig removed the WIP label May 6, 2019
jrodewig added a commit that referenced this pull request May 6, 2019
* [DOCS] Restructure `term` query docs.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 7, 2019
* elastic/master: (414 commits)
  Add tasks to build Docker build context artifacts (elastic#41819)
  Replace more uses of immutable map builder (elastic#41823)
  Force selection of calendar or fixed intervals in date histo agg (elastic#33727)
  Switch run task to use real distro (elastic#41590)
  Clarify that path_match also considers object fields. (elastic#41658)
  [DOCS] remove 'es.scripting.update.ctx_in_params' system property for 7.0 (elastic#41643)
  Clarify _doc is a permanent part of certain document APIs. (elastic#41727)
  Remove the jdk directory to save space on bwc tests (elastic#41743)
  Fix full text queries test that start with now (elastic#41854)
  Remove `nonApplicationWrite` from `SSLDriver` (elastic#41829)
  SQL: [Docs] Add example for custom bucketing with CASE (elastic#41787)
  Cleanup Bulk Delete Exception Logging (elastic#41693)
  [DOCS] Rewrite `term` query docs for new format (elastic#41498)
  Mute PermissionsIT#testWhen[...]ByILMPolicy (elastic#41858)
  ReadOnlyEngine assertion fix (elastic#41842)
  [ML] addresses preview bug, and adds check to PUT (elastic#41803)
  Fix javadoc in WrapperQueryBuilder
  Testsclusters use seprate configurations per version (elastic#41504)
  Skip explain fetch sub phase when request holds only suggestions (elastic#41739)
  remove unused import
  ...
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
* [DOCS] Restructure `term` query docs.
@jrodewig jrodewig added the v6.8.1 label Jun 6, 2019
jrodewig added a commit that referenced this pull request Jun 6, 2019
* [DOCS] Restructure `term` query docs.
jrodewig added a commit that referenced this pull request Jun 6, 2019
@jrodewig jrodewig removed the v6.8.1 label Jun 6, 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.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants