-
Notifications
You must be signed in to change notification settings - Fork 65
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 update document with knnn_vector size not matching issue #208
Fix update document with knnn_vector size not matching issue #208
Conversation
Signed-off-by: zane-neo <zaniu@amazon.com>
@zane-neo before PR can be reviewed, can you check why gradle checks are failing? |
Signed-off-by: zane-neo <zaniu@amazon.com>
Added commons-lang3 back, in my local even I removed this dependency the project can be compiled successfully, and adding back this can cause jar hell issue when building neural-search plugin, but it seems have to add it back to pass the build. |
@zane-neo the build and test gradle check is still failing. Please fix them. |
Signed-off-by: zane-neo <zaniu@amazon.com>
Signed-off-by: zane-neo <zaniu@amazon.com>
Signed-off-by: zane-neo <zaniu@amazon.com>
@navneet1v opensearch core changed some classes breaking the code build, I've created a PR to fix this issue in k-NN repo: opensearch-project/k-NN#958, please help take a look. Thanks. |
Signed-off-by: zane-neo <zaniu@amazon.com>
Signed-off-by: zane-neo <zaniu@amazon.com>
Signed-off-by: zane-neo <zaniu@amazon.com>
@@ -119,7 +117,7 @@ void appendVectorFieldsToDocument(IngestDocument ingestDocument, Map<String, Obj | |||
Objects.requireNonNull(vectors, "embedding failed, inference returns null result!"); | |||
log.debug("Text embedding result fetched, starting build vector output!"); | |||
Map<String, Object> textEmbeddingResult = buildTextEmbeddingResult(knnMap, vectors, ingestDocument.getSourceAndMetadata()); | |||
textEmbeddingResult.forEach(ingestDocument::appendFieldValue); | |||
textEmbeddingResult.forEach(ingestDocument::setFieldValue); |
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.
can we rename the function from appendVectorFieldsToDocument to setVectorFieldsToDocument, as we are not appending the values
…cument Signed-off-by: zane-neo <zaniu@amazon.com>
Signed-off-by: zane-neo <zaniu@amazon.com>
Build is failing caused by opensearch core breaking change, and is fixed by k-NN team in this PR: opensearch-project/k-NN#963. |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-208-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 95993717e59b31bcd0facc76b0c702aa6857b8f8
# Push it to GitHub
git push --set-upstream origin backport/backport-208-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x Then, create a pull request where the |
…rch-project#208) * Fix update document with knnn_vector size not matching issue Signed-off-by: zane-neo <zaniu@amazon.com> * Add apache common lang3 back to fix PR gradle build failure Signed-off-by: zane-neo <zaniu@amazon.com> * Fix PR check failure Signed-off-by: zane-neo <zaniu@amazon.com> * Fix PR check failure Signed-off-by: zane-neo <zaniu@amazon.com> * Fix PR check jar hell failure Signed-off-by: zane-neo <zaniu@amazon.com> * Add apache common lang back to fix the PR check Signed-off-by: zane-neo <zaniu@amazon.com> * Fix register model group failure in IT Signed-off-by: zane-neo <zaniu@amazon.com> * format code Signed-off-by: zane-neo <zaniu@amazon.com> * Rename the method appendVectorFieldsToDocument to setVectorFieldsToDocument Signed-off-by: zane-neo <zaniu@amazon.com> * format code Signed-off-by: zane-neo <zaniu@amazon.com> --------- Signed-off-by: zane-neo <zaniu@amazon.com> (cherry picked from commit 9599371)
Looks like this PR directly merged to 2.x branch instead of main, I am adding 2.9 release tag for the PR and adding to 2.9 release note. @zane-neo @ylwu-amzn |
@zane-neo this PR also needs to be raised on main branch. Please going forward follow the below practice. |
Description
This PR fixed the issue when updating a document with knn_vector field, the update fails because the vector dimension is not matching the required dimension.
Issues Resolved
#207
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.