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 or update boost prameter for term-level queries #6700

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

imamuray
Copy link
Contributor

@imamuray imamuray commented Mar 17, 2024

Description

Term-level queries can set boost parameter, but some of them are not mentioned in the documentation.

Term-level query Add or Update boost
exists Add
fuzzy Add
ids Add
prefix Add
range Update
regexp Add
term Update
terms Update
term_set Add
wildcard Update

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@imamuray imamuray changed the title Add to boost parameter to the prefix query Add boost parameter to the prefix query Mar 17, 2024
@hdhalter hdhalter added the 3 - Tech review PR: Tech review in progress label Mar 18, 2024
@hdhalter
Copy link
Contributor

@owaiskazi19 - Can you please review this contribution from a technical standpoint? Thanks!

@hdhalter hdhalter added the backport 2.12 PR: Backport label for 2.12 label Mar 18, 2024
@imamuray
Copy link
Contributor Author

imamuray commented Mar 19, 2024

I investigated the situation regarding the boost parameter for term-level queries, including prefix. Below is a table indicating where the boost is parsed and whether the boost parameter is explicitly documented:

Term-level query Pasing in code boost in doc?
exists ExistsQueryBuilder.java L131 no
fuzzy FuzzyQueryBuilder.java L294 no
ids ?
found boost in test but not asserted
no
prefix PrefixQueryBuilder.java L178 no
range already stated in doc
regexp RegexpQueryBuilder.java L237 no
term already stated in doc
terms already stated in doc
term_set TermsSetQueryBuilder.java L215 no
wildcard already stated in doc

Should I also add the boost parameter to the term-level query documentation other than prefix in this pull request? In that case, I will change the title and make some commits.

@@ -63,8 +63,9 @@ The `<field>` accepts the following parameters. All parameters except `value` ar
Parameter | Data type | Description
:--- | :--- | :---
`value` | String | The term to search for in the field specified in `<field>`.
`boost` | Floating-point | Boosts the query by the given multiplier. Useful for searches that contain more than one query. Values in the [0, 1) range decrease relevance, and values greater than 1 increase relevance. Default is `1`.
Copy link
Member

@owaiskazi19 owaiskazi19 Mar 19, 2024

Choose a reason for hiding this comment

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

@imamuray all PrefixQueryBuilder supports are value, rewrite and caseInsensitive. Any reason for this change?
Are you trying to mention the boost keyword of the index mappings here?

Copy link
Member

Choose a reason for hiding this comment

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

I missed the comment here. Ignore my previous comment. The boost here is keyword field type defined here. To make it synchronize across all the term queries we should either add boost in all of them or remove from few of them.

If we want to add it in all of them, we can use the same description

A floating-point value that specifies the weight of this field toward the relevance score. Values above 1.0 increase the field’s relevance. Values between 0.0 and 1.0 decrease the field’s relevance. Default is 1.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@owaiskazi19 Thank you for the review.
I have added boost to all term-level queries. 080f57c

@imamuray imamuray changed the title Add boost parameter to the prefix query Add or update boost prameter for term-level queries Mar 21, 2024
@hdhalter hdhalter added 4 - Doc review PR: Doc review in progress and removed 3 - Tech review PR: Tech review in progress labels Mar 26, 2024
Copy link
Collaborator

@kolchfa-aws kolchfa-aws left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you, @imamuray!

@kolchfa-aws kolchfa-aws added backport 2.13 PR: Backport label for 2.13 and removed backport 2.12 PR: Backport label for 2.12 labels Apr 2, 2024
@kolchfa-aws kolchfa-aws merged commit 9ff5db3 into opensearch-project:main Apr 2, 2024
5 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 2, 2024
* Add to boost parameter to the prefix query

Signed-off-by: Yuki Imamura <[email protected]>

* Add or update boost prameter for term-level queries

Signed-off-by: Yuki Imamura <[email protected]>

---------

Signed-off-by: Yuki Imamura <[email protected]>
(cherry picked from commit 9ff5db3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kolchfa-aws pushed a commit that referenced this pull request Apr 2, 2024
* Add to boost parameter to the prefix query



* Add or update boost prameter for term-level queries



---------


(cherry picked from commit 9ff5db3)

Signed-off-by: Yuki Imamura <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Doc review PR: Doc review in progress backport 2.13 PR: Backport label for 2.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants