HSEARCH-4950 TOFIXUP: Update knn restrictions for Elasticsearch

hibernate · Jan 17, 2024 · 8ee07df · 8ee07df
1 parent aeeb486
commit 8ee07df
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 14 deletions.
diff --git a/...ate/search/backend/elasticsearch/search/predicate/impl/ElasticsearchBooleanPredicate.java b/...ate/search/backend/elasticsearch/search/predicate/impl/ElasticsearchBooleanPredicate.java
@@ -65,10 +65,11 @@ private ElasticsearchBooleanPredicate(Builder builder) {
 
 	@Override
 	public void checkNestableWithin(PredicateNestingContext context) {
-		checkAcceptableWithin( context, mustClauses );
-		checkAcceptableWithin( context, shouldClauses );
-		checkAcceptableWithin( context, filterClauses );
-		checkAcceptableWithin( context, mustNotClauses );
+		PredicateNestingContext updatedContext = context.rejectKnn();
+		checkAcceptableWithin( updatedContext, mustClauses );
+		checkAcceptableWithin( updatedContext, shouldClauses );
+		checkAcceptableWithin( updatedContext, filterClauses );
+		checkAcceptableWithin( updatedContext, mustNotClauses );
 	}
 
 	@Override
@@ -241,20 +242,12 @@ public void should(SearchPredicate clause) {
 
 			ElasticsearchSearchPredicate elasticsearchClause = ElasticsearchSearchPredicate.from( scope, clause );
 			elasticsearchClause.checkNestableWithin(
-					!hasNonShouldClause() && maybeKnnClause( clause )
+					!hasNonShouldClause()
 							? PredicateNestingContext.acceptsKnn()
 							: PredicateNestingContext.doesNotAcceptKnn() );
 			shouldClauses.add( elasticsearchClause );
 		}
 
-		private boolean maybeKnnClause(SearchPredicate clause) {
-			return clause instanceof ElasticsearchKnnPredicate
-					|| ( clause instanceof ElasticsearchNamedPredicate
-					// TODO:
-					// && clause.providedPredicate instanceof ElasticsearchKnnPredicate
-					);
-		}
-
 		@Override
 		public void filter(SearchPredicate clause) {
 			if ( filterClauses == null ) {

diff --git a/...hibernate/search/backend/elasticsearch/search/predicate/impl/PredicateNestingContext.java b/...hibernate/search/backend/elasticsearch/search/predicate/impl/PredicateNestingContext.java
@@ -45,4 +45,13 @@ public boolean acceptsKnnClause() {
 		return acceptsKnnClause;
 	}
 
+	public PredicateNestingContext rejectKnn() {
+		if ( !acceptsKnnClause ) {
+			return this;
+		}
+		if ( nestedPath == null ) {
+			return DOES_NOT_ACCEPT_KNN;
+		}
+		return new PredicateNestingContext( nestedPath, false );
+	}
 }
diff --git a/...ch/integrationtest/backend/elasticsearch/search/ElasticsearchKnnPredicateSpecificsIT.java b/...ch/integrationtest/backend/elasticsearch/search/ElasticsearchKnnPredicateSpecificsIT.java
@@ -217,6 +217,8 @@ void knnPredicateInWrongPlace_addingPrebuiltKnn() {
 					// cannot add a knn through should as we already have a non-should clause
 					.should( knn ) );
 
+			// nested knn predicates are not allowed
+			knnPredicateInWrongPlace( () -> scope.predicate().nested( "object" ).add( knn ) );
 
 			// we add multiple clauses to prevent "optimizations" leading to bool predicate being replaced by a simple knn predicate
 			SearchPredicate inlineBoolKnnInShould = scope.predicate().bool().should( knn )
@@ -239,8 +241,10 @@ void knnPredicateInWrongPlace_addingPrebuiltKnn() {
 				knnPredicateInWrongPlace(
 						() -> scope.predicate().knn( 10 ).field( "location" ).matching( 50.0f, 50.0f )
 								.filter( boolKnnInShould ) );
-			}
 
+				// nested knn predicates are not allowed
+				knnPredicateInWrongPlace( () -> scope.predicate().nested( "object" ).add( boolKnnInShould ) );
+			}
 		}
 	}