-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Add index_options to semantic_text field mappings #119967
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 index_options to semantic_text field mappings #119967
Conversation
e096a61
to
342d769
Compare
342d769
to
d822301
Compare
Hi @kderusso, I've created a changelog YAML for you. |
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
Pinging @elastic/search-eng (Team:SearchOrg) |
Pinging @elastic/search-relevance (Team:Search - Relevance) |
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.
Good start to this! I have a bunch of comments, but they're mostly interrelated, so it's not as much as it seems.
...ugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextField.java
Outdated
Show resolved
Hide resolved
...ugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextField.java
Outdated
Show resolved
Hide resolved
...ugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextField.java
Outdated
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml
Outdated
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml
Outdated
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml
Show resolved
Hide resolved
...e/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml
Show resolved
Hide resolved
...ugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextField.java
Outdated
Show resolved
Hide resolved
This reverts commit aedfafe.
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 left one comment regarding validation but this feels very close @kderusso !
...nference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapper.java
Outdated
Show resolved
Hide resolved
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
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.
Nice implementation and tests, left a few nitpick comments.
...nce/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java
Show resolved
Hide resolved
...nce/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java
Show resolved
Hide resolved
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.
💔 Backport failed
You can use sqren/backport to manually backport by running |
* Add index_options parameter to semantic_text field mapping * Cleanup & tests * Update docs * Update docs/changelog/119967.yaml * Addressed some PR feedbak * Update yaml tests * Refactoring * Cleanup * Fix some tests * Hack in inferring text_embedding task type from index options * [CI] Auto commit changes from spotless * Fix error inferring model settings * Update docs * Update tests * Update docs/reference/mapping/types/semantic-text.asciidoc Co-authored-by: Mike Pellegrini <[email protected]> * Address some minor PR feedback * Remove partial model_settings with inferred task type * Cleanup * Remove unnecessary changes * Fix errors from merge * [CI] Auto commit changes from spotless * Cleanup * Checkpoint, saving changes before merge * Update parsing * [CI] Auto commit changes from spotless * Stash changes * Fix compile errors * [CI] Auto commit changes from spotless * Cleanup error * fix test * fix test * Fix another test * A bit of cleanup * Fix tests * Spotless * Respect index options if set over defaults * Cleanup * [CI] Auto commit changes from spotless * Support updating to compatible versions, add some cleanup and validation * Remove test that can't be done here - needs to be unit test * Add validation * Cleanup * Fix some yaml tests * [CI] Auto commit changes from spotless * Happy path early index validation works now; edge cases surrounding default BBQ remain * Always emit index options, even when using defaults * Minor cleanup * Fix test compilation failures * Fix some tests * Continue to iterate on test failures * Remove index options from inference field metadata as it is only needed at field creation time * Fix some tests * Remove transport version, no longer needed * Fix yaml tests * Add tests * IndexOptions don't need to implement Writeable * [CI] Auto commit changes from spotless * Refactor - move SemanticTextIndexOptions * Remove writeable * Move index_options parsing to semantic text field mapper * Cleanup * Fix test compilation issue * Cleanup * Remove whitespace * Remove writeables from index options * Disable merging null options? * Add docs * [CI] Auto commit changes from spotless * Revert "Disable merging null options?" This reverts commit 2ef8b1d. * Remove default serialization * Include default index option type to defaults * [CI] Auto commit changes from spotless * Go back to allowing null updateS * Cleanup * Fix validation error * Revert "Include default index option type to defaults" This reverts commit b08e2a1. * Update tests * Revert "Update tests" This reverts commit aedfafe. * Better fix for null inputs * Remove redundant merge validation --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Mike Pellegrini <[email protected]> (cherry picked from commit 813814b) # Conflicts: # docs/reference/elasticsearch/mapping-reference/semantic-text.md # server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapper.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
Had to create a manual backport: #129626 |
…129626) * Add index_options to semantic_text field mappings (#119967) * Add index_options parameter to semantic_text field mapping * Cleanup & tests * Update docs * Update docs/changelog/119967.yaml * Addressed some PR feedbak * Update yaml tests * Refactoring * Cleanup * Fix some tests * Hack in inferring text_embedding task type from index options * [CI] Auto commit changes from spotless * Fix error inferring model settings * Update docs * Update tests * Update docs/reference/mapping/types/semantic-text.asciidoc Co-authored-by: Mike Pellegrini <[email protected]> * Address some minor PR feedback * Remove partial model_settings with inferred task type * Cleanup * Remove unnecessary changes * Fix errors from merge * [CI] Auto commit changes from spotless * Cleanup * Checkpoint, saving changes before merge * Update parsing * [CI] Auto commit changes from spotless * Stash changes * Fix compile errors * [CI] Auto commit changes from spotless * Cleanup error * fix test * fix test * Fix another test * A bit of cleanup * Fix tests * Spotless * Respect index options if set over defaults * Cleanup * [CI] Auto commit changes from spotless * Support updating to compatible versions, add some cleanup and validation * Remove test that can't be done here - needs to be unit test * Add validation * Cleanup * Fix some yaml tests * [CI] Auto commit changes from spotless * Happy path early index validation works now; edge cases surrounding default BBQ remain * Always emit index options, even when using defaults * Minor cleanup * Fix test compilation failures * Fix some tests * Continue to iterate on test failures * Remove index options from inference field metadata as it is only needed at field creation time * Fix some tests * Remove transport version, no longer needed * Fix yaml tests * Add tests * IndexOptions don't need to implement Writeable * [CI] Auto commit changes from spotless * Refactor - move SemanticTextIndexOptions * Remove writeable * Move index_options parsing to semantic text field mapper * Cleanup * Fix test compilation issue * Cleanup * Remove whitespace * Remove writeables from index options * Disable merging null options? * Add docs * [CI] Auto commit changes from spotless * Revert "Disable merging null options?" This reverts commit 2ef8b1d. * Remove default serialization * Include default index option type to defaults * [CI] Auto commit changes from spotless * Go back to allowing null updateS * Cleanup * Fix validation error * Revert "Include default index option type to defaults" This reverts commit b08e2a1. * Update tests * Revert "Update tests" This reverts commit aedfafe. * Better fix for null inputs * Remove redundant merge validation --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Mike Pellegrini <[email protected]> (cherry picked from commit 813814b) # Conflicts: # docs/reference/elasticsearch/mapping-reference/semantic-text.md # server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapper.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java * Fix errors in backport merge
Adds
index_options
support forsemantic_text
fields using dense models.Example: