Disallow kNN searches on nested vector fields#79403
Disallow kNN searches on nested vector fields#79403jtibshirani merged 2 commits intoelastic:masterfrom
Conversation
| search: | ||
| rest_total_hits_as_int: true | ||
| index: test-index | ||
| knn_search: |
There was a problem hiding this comment.
There are extra changes here because I caught and fixed some mistakes in these REST tests.
|
Pinging @elastic/es-search (Team:Search) |
mayya-sharipova
left a comment
There was a problem hiding this comment.
@jtibshirani Thanks, this LGTM!
| name: moose.jpg | ||
| vector: [-0.5, 100.0, -13, 14.8, -156.0] | ||
| comments: | ||
| - body: "what a great moose" |
There was a problem hiding this comment.
cool descriptions!!!
There was a problem hiding this comment.
hehe, I got bored with my usual test examples :)
| * `bar.baz`, then calling this method with `foo.bar.baz` will return | ||
| * the path `foo`, skipping over the object-but-not-nested `foo.bar` | ||
| */ | ||
| public String getNestedParent(String path) { |
There was a problem hiding this comment.
it seems that the previous getNestedParent will return null if path is not an objectMapper? But from how this method was used, it looks like this check is not important. So this change LGTM.
There was a problem hiding this comment.
Right, this behavior is different but I also couldn't find any place where this check was important.
romseygeek
left a comment
There was a problem hiding this comment.
LGTM! I have a separate issue to clean up all these nested methods but this is good for now.
|
Thank you both for reviews. |
The new kNN endpoint currently doesn't support searches on nested fields. This
PR updates the validation logic to detect this case and throw a clear error. It
also adds tests for kNN search when there are nested documents.
Relates to #78473.