-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Fix Semantic Query Rewrite Interception Drops Boosts #129282
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
Fix Semantic Query Rewrite Interception Drops Boosts #129282
Conversation
Hi @Samiul-TheSoccerFan, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @Samiul-TheSoccerFan ! Can we please add some tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to see a solution based on copy constructors. Delegating the responsibility of generating a complete copy to the caller is error-prone and hard to maintain, which is how this class of bugs got introduced in the first place.
@elasticmachine update branch |
merge conflict between base and head |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to adjust the error ranges and address @kderusso's comments, everything else looks good.
...est/java/org/elasticsearch/index/query/SemanticSparseVectorQueryRewriteInterceptorTests.java
Outdated
Show resolved
Hide resolved
...inference/src/yamlRestTest/resources/rest-api-spec/test/inference/45_semantic_text_match.yml
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/46_semantic_text_sparse_vector.yml
Show resolved
Hide resolved
...ain/java/org/elasticsearch/xpack/inference/queries/SemanticMatchQueryRewriteInterceptor.java
Show resolved
Hide resolved
@elasticmachine update branch |
@elasticmachine update branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM provided CI passes; thanks for all the iterations on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
...c/test/java/org/elasticsearch/index/query/SemanticKnnVectorQueryRewriteInterceptorTests.java
Show resolved
Hide resolved
...inference/src/yamlRestTest/resources/rest-api-spec/test/inference/45_semantic_text_match.yml
Show resolved
Hide resolved
@elasticmachine update branch |
💔 Backport failed
You can use sqren/backport to manually backport by running |
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
* fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java
…#131473) * Fix Semantic Query Rewrite Interception Drops Boosts (#129282) * fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java * update test to call the correct constructor
…131471) * Fix Semantic Query Rewrite Interception Drops Boosts (#129282) * fix boosting for knn * Fixing for match query * fixing for match subquery * fix for sparse vector query boost * fix linting issues * Update docs/changelog/129282.yaml * update changelog * Copy constructor with match query * util function to create sparseVectorBuilder for sparse query * util function for knn query to support boost * adding unit tests for all intercepted query terms * Adding yaml test for match,sparse, and knn * Adding queryname support for nested query * fix code styles * Fix failed yaml tests * Update docs/changelog/129282.yaml * update yaml tests to expand test scenarios * Updating knn to copy constructor * adding yaml tests for multiple indices * refactoring match query to adjust boost and queryname and move to copy constructor * refactoring sparse query to adjust boost and queryname and move to copy constructor * [CI] Auto commit changes from spotless * Refactor sparse vector to adjust boost and queryname in the top level * Refactor knn vector to adjust boost and queryname in the top level * fix knn combined query * fix unit tests * fix lint issues * remove unused code * Update inference feature name * Remove double boosting issue from match * Fix double boosting in match test yaml file * move to bool level for match semantic boost * fix double boosting for sparse vector * fix double boosting for sparse vector in yaml test * fix knn combined query * fix knn combined query * fix sparse combined query * fix knn yaml test for combined query * refactoring unit tests * linting * fix match query unit test * adding copy constructor for match query * refactor copy match builder to intercepter * [CI] Auto commit changes from spotless * fix unit tests * update yaml tests * fix match yaml test * fix yaml tests with 4 digits error margin * unit tests are now more randomized --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit e2bb47c) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java * update test to call the correct constructor
Closes ##128696
Match query: boost only on inference field
Match query: boost on both semantic_text and text fields
KNN
Sparse