Skip to content

[DOCS] Rewrite prefix query docs #41955

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

Merged
merged 11 commits into from
Jul 29, 2019
Merged

[DOCS] Rewrite prefix query docs #41955

merged 11 commits into from
Jul 29, 2019

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented May 8, 2019

Changes

  • Rewrites description of prefix query
  • Include boost and rewrite parameters in example request
  • Adds parameters sections

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

Any and all feedback welcome!

Before

Before image Screen Shot 2019-05-08 at 11 34 54 AM

After

After image Screen Shot 2019-05-08 at 11 36 13 AM

@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.0 v7.3.0 labels May 8, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jrodewig jrodewig removed the v6.8.2 label Jun 28, 2019
@jrodewig
Copy link
Contributor Author

jrodewig commented Jul 3, 2019

@elasticmachine run elasticsearch-ci/docs-check

@jrodewig
Copy link
Contributor Author

jrodewig commented Jul 3, 2019

@elasticmachine run elasticsearch-ci/docbldesx

@jrodewig jrodewig added the v7.4.0 label Jul 3, 2019
@jpountz jpountz removed the v7.3.0 label Jul 3, 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.

Nice rewrite again, I left two small questions.

// CONSOLE

A boost can also be associated with the query:
[[prefix-query-top-level-params]]
==== Top-level parameters for `term`
Copy link
Member

Choose a reason for hiding this comment

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

Should this be "prefix" instead of "term"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Copy/paste error. Fixed with 1e6ca49.

"prefix": {
"user": {
"value": "ki",
"rewrite": "constant_score"
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if this parameter should be included in the "default" example since IMHO its pretty much an advanced parameter and mentioned later (also, constant_score should be the default, so not necessary here).
Did you consider also documenting the previous "short" form or do we want to discourage ppl. from using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question!

I've been using the longer form requests because it's easier to delineate additional parameters. This is less of an issue with the prefix query, but I used the longer form so it's consistent with other queries. However, I'm open to changing this if wanted.

I've removed the rewrite parameter and added a "short request example" with 1e6ca49 and d58de59.

@jrodewig
Copy link
Contributor Author

jrodewig commented Jul 8, 2019

Thanks again for your review, @cbuescher. I've made a few changes in response to your feedback.

Let me know if you have any other thoughts.

@romseygeek
Copy link
Contributor

It might be worth also referring to the index-prefixes option on text fields, which speed up prefix queries considerably.

@jrodewig
Copy link
Contributor Author

Great suggestion @romseygeek. I've added a section for index_prefixes with b20e099.

@jrodewig jrodewig requested a review from cbuescher July 24, 2019 15:10
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 for the changes, LGTM

@jrodewig jrodewig merged commit a5df840 into elastic:master Jul 29, 2019
jrodewig added a commit that referenced this pull request Jul 29, 2019
jrodewig added a commit that referenced this pull request Jul 29, 2019
jrodewig added a commit that referenced this pull request Jul 29, 2019
jrodewig added a commit that referenced this pull request Jul 29, 2019
jrodewig added a commit that referenced this pull request Jul 29, 2019
jkakavas pushed a commit that referenced this pull request Jul 31, 2019
@jrodewig jrodewig deleted the prefix-query-rewrite branch January 29, 2020 14:08
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.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants