Skip to content

Commit

Permalink
Disallow kNN searches on nested vector fields (elastic#79403)
Browse files Browse the repository at this point in the history
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 elastic#78473.
  • Loading branch information
jtibshirani authored Oct 20, 2021
1 parent 58abbe9 commit 97c62a7
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ public void validate(MappingLookup mappers) {
throw new MapperParsingException("Invalid [path] value [" + path + "] for field alias [" +
name() + "]: an alias cannot refer to another alias.");
}
String aliasScope = mappers.getNestedScope(name);
String pathScope = mappers.getNestedScope(path);
String aliasScope = mappers.getNestedParent(name);
String pathScope = mappers.getNestedParent(path);

if (Objects.equals(aliasScope, pathScope) == false) {
StringBuilder message = new StringBuilder("Invalid [path] value [" + path + "] for field alias [" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ public final void validate(MappingLookup mappers) {
throw new IllegalArgumentException("[copy_to] may not be used to copy from a multi-field: [" + this.name() + "]");
}

final String sourceScope = mappers.getNestedScope(this.name());
final String sourceScope = mappers.getNestedParent(this.name());
for (String copyTo : this.copyTo().copyToFields()) {
if (mappers.isMultiField(copyTo)) {
throw new IllegalArgumentException("[copy_to] may not be used to copy to a multi-field: [" + copyTo + "]");
Expand All @@ -299,7 +299,7 @@ public final void validate(MappingLookup mappers) {
throw new IllegalArgumentException("Cannot copy to field [" + copyTo + "] since it is mapped as an object");
}

final String targetScope = mappers.getNestedScope(copyTo);
final String targetScope = mappers.getNestedParent(copyTo);
checkNestedScopeCompatibility(sourceScope, targetScope);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,14 @@ public boolean isObjectField(String field) {
return objectMappers.containsKey(field);
}

public String getNestedScope(String path) {
/**
* Given a nested object path, returns the path to its nested parent
*
* In particular, if a nested field `foo` contains an object field
* `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) {
for (String parentPath = parentObject(path); parentPath != null; parentPath = parentObject(parentPath)) {
ObjectMapper objectMapper = objectMappers.get(parentPath);
if (objectMapper != null && objectMapper.isNested()) {
Expand Down Expand Up @@ -466,34 +473,4 @@ public List<NestedObjectMapper> getNestedParentMappers() {
}
return parents;
}

/**
* Given a nested object path, returns the path to its nested parent
*
* In particular, if a nested field `foo` contains an object field
* `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) {
ObjectMapper mapper = objectMappers().get(path);
if (mapper == null) {
return null;
}
if (path.contains(".") == false) {
return null;
}
do {
path = path.substring(0, path.lastIndexOf("."));
mapper = objectMappers().get(path);
if (mapper == null) {
return null;
}
if (mapper.isNested()) {
return path;
}
if (path.contains(".") == false) {
return null;
}
} while(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,26 @@ setup:
dims: 5
index: true
similarity: l2_norm
comments:
type: nested
properties:
body:
type: text
vector:
type: dense_vector
dims: 5
index: true
similarity: l2_norm

- do:
index:
index: test
body:
name: cow.jpg
vector: [230.0, 300.33, -34.8988, 15.555, -200.0]
comments:
- body: "free the cows"
vector: [0.75, 100.0, 0.33, 16.2, -10.2]

- do:
index:
Expand All @@ -28,6 +42,9 @@ setup:
body:
name: moose.jpg
vector: [-0.5, 100.0, -13, 14.8, -156.0]
comments:
- body: "what a great moose"
vector: [11.4, 99.0, 1.55, -2.9, -10.2]

- do:
index:
Expand Down Expand Up @@ -63,19 +80,20 @@ setup:
"Test nonexistent field":
- do:
catch: bad_request
search:
rest_total_hits_as_int: true
index: test-index
knn_search:
index: test
body:
query:
knn:
field: nonexistent
query_vector: [ -0.5, 90.0, -10, 14.8, -156.0 ]
num_candidates: 1
- match: { error.root_cause.0.type: "illegal_argument_exception" }
fields: [ "name" ]
knn:
field: nonexistent
query_vector: [ -0.5, 90.0, -10, 14.8, -156.0 ]
k: 2
num_candidates: 3
- match: { error.root_cause.0.type: "query_shard_exception" }
- match: { error.root_cause.0.reason: "failed to create query: field [nonexistent] does not exist in the mapping" }

---
"Direct knn queries are disallowed":
"Direct kNN queries are disallowed":
- do:
catch: bad_request
search:
Expand All @@ -88,3 +106,20 @@ setup:
query_vector: [ -0.5, 90.0, -10, 14.8, -156.0 ]
num_candidates: 1
- match: { error.root_cause.0.type: "illegal_argument_exception" }
- match: { error.root_cause.0.reason: "[knn] queries cannot be provided directly, use the [_knn_search] endpoint instead" }

---
"kNN searches on nested fields are disallowed":
- do:
catch: bad_request
knn_search:
index: test
body:
fields: [ "nonexistent" ]
knn:
field: comments.vector
query_vector: [ -0.5, 90.0, -10, 14.8, -156.0 ]
k: 2
num_candidates: 3
- match: { error.root_cause.0.type: "query_shard_exception" }
- match: { error.root_cause.0.reason: "failed to create query: [knn] queries are not supported on nested fields" }
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,17 @@ protected Query doToQuery(SearchExecutionContext context) {
+ DenseVectorFieldMapper.CONTENT_TYPE + "] fields");
}

if (context.getNestedParent(fieldType.name()) != null) {
throw new IllegalArgumentException("[" + NAME + "] queries are not supported on nested fields");
}

DenseVectorFieldType vectorFieldType = (DenseVectorFieldType) fieldType;
if (queryVector.length != vectorFieldType.dims()) {
throw new IllegalArgumentException("the query vector has a different dimension [" + queryVector.length + "] "
+ "than the index vectors [" + vectorFieldType.dims() + "]");
}
if (vectorFieldType.isSearchable() == false) {
throw new IllegalArgumentException("[" + "[" + NAME + "] queries are not supported if [index] is disabled");
throw new IllegalArgumentException("[" + NAME + "] queries are not supported if [index] is disabled");
}
return new KnnVectorQuery(fieldType.name(), queryVector, numCands);
}
Expand Down

0 comments on commit 97c62a7

Please sign in to comment.