HSEARCH-4217 Move nested document compatibility checks for simpleQuer…

…yString to the engine
hibernate · Jun 14, 2021 · c41c05b · c41c05b
1 parent 35c0bf4
commit c41c05b
Show file tree

Hide file tree

Showing 11 changed files with 77 additions and 173 deletions.
diff --git a/...sticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java b/...sticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java
@@ -525,13 +525,6 @@ SearchException elasticsearchIndexNameAndAliasesMatchMultipleIndexes(URLEncodedS
 			+ " Set the schema management strategy via the property 'hibernate.search.schema_management.strategy' instead.")
 	SearchException lifecycleStrategyMovedToMapper();
 
-	@Message(id = ID_OFFSET + 99,
-			value = "Invalid target fields for simple-query-string predicate:"
-					+ " fields [%1$s, %3$s] are in different nested documents [%2$s, %4$s]."
-					+ " All fields targeted by a simple-query-string predicate must be in the same document.")
-	SearchException simpleQueryStringSpanningMultipleNestedPaths(String fieldPath1, String nestedPath1,
-			String fieldPath2, String nestedPath2);
-
 	@Message(id = ID_OFFSET + 100,
 			value = "Invalid sort mode: %1$s. This sort mode is not supported for fields in nested documents.")
 	SearchException invalidSortModeAcrossNested(SortMode mode, @Param EventContext context);

diff --git a/...lasticsearch/scope/model/impl/ElasticsearchDifferentNestedObjectCompatibilityChecker.java b/...lasticsearch/scope/model/impl/ElasticsearchDifferentNestedObjectCompatibilityChecker.java
diff --git a/.../backend/elasticsearch/search/predicate/impl/ElasticsearchSimpleQueryStringPredicate.java b/.../backend/elasticsearch/search/predicate/impl/ElasticsearchSimpleQueryStringPredicate.java
@@ -16,10 +16,10 @@
 import org.hibernate.search.backend.elasticsearch.gson.impl.JsonAccessor;
 import org.hibernate.search.backend.elasticsearch.gson.impl.JsonObjectAccessor;
 import org.hibernate.search.backend.elasticsearch.lowlevel.index.analysis.impl.AnalyzerConstants;
-import org.hibernate.search.backend.elasticsearch.scope.model.impl.ElasticsearchDifferentNestedObjectCompatibilityChecker;
 import org.hibernate.search.backend.elasticsearch.search.common.impl.ElasticsearchSearchIndexScope;
 import org.hibernate.search.backend.elasticsearch.types.predicate.impl.ElasticsearchSimpleQueryStringPredicateBuilderFieldState;
 import org.hibernate.search.engine.search.common.BooleanOperator;
+import org.hibernate.search.engine.search.common.spi.SearchIndexSchemaElementContextHelper;
 import org.hibernate.search.engine.search.predicate.SearchPredicate;
 import org.hibernate.search.engine.search.predicate.dsl.SimpleQueryFlag;
 import org.hibernate.search.engine.search.predicate.spi.SimpleQueryStringPredicateBuilder;
@@ -53,10 +53,10 @@ public class ElasticsearchSimpleQueryStringPredicate extends AbstractElasticsear
 
 	ElasticsearchSimpleQueryStringPredicate(Builder builder) {
 		super( builder );
-		nestedPathHierarchy = builder.nestedCompatibilityChecker.getNestedPathHierarchy();
-		fieldPaths = new ArrayList<>( builder.fields.keySet() );
+		nestedPathHierarchy = builder.firstFieldState.field().nestedPathHierarchy();
+		fieldPaths = new ArrayList<>( builder.fieldStates.keySet() );
 		fieldNameAndBoosts = new ArrayList<>();
-		for ( ElasticsearchSimpleQueryStringPredicateBuilderFieldState fieldContext : builder.fields.values() ) {
+		for ( ElasticsearchSimpleQueryStringPredicateBuilderFieldState fieldContext : builder.fieldStates.values() ) {
 			fieldNameAndBoosts.add( fieldContext.build() );
 		}
 		defaultOperator = builder.defaultOperator;
@@ -141,16 +141,15 @@ private String getFlagName(SimpleQueryFlag flag) {
 
 	public static class Builder extends AbstractBuilder implements SimpleQueryStringPredicateBuilder {
 
-		private final Map<String, ElasticsearchSimpleQueryStringPredicateBuilderFieldState> fields = new LinkedHashMap<>();
+		private ElasticsearchSimpleQueryStringPredicateBuilderFieldState firstFieldState;
+		private final Map<String, ElasticsearchSimpleQueryStringPredicateBuilderFieldState> fieldStates = new LinkedHashMap<>();
 		private JsonPrimitive defaultOperator = OR_OPERATOR_KEYWORD_JSON;
 		private String simpleQueryString;
 		private String analyzer;
 		private EnumSet<SimpleQueryFlag> flags;
-		private ElasticsearchDifferentNestedObjectCompatibilityChecker nestedCompatibilityChecker;
 
 		Builder(ElasticsearchSearchIndexScope scope) {
 			super( scope );
-			this.nestedCompatibilityChecker = ElasticsearchDifferentNestedObjectCompatibilityChecker.empty( scope );
 		}
 
 		@Override
@@ -172,13 +171,19 @@ public void flags(Set<SimpleQueryFlag> flags) {
 
 		@Override
 		public FieldState field(String absoluteFieldPath) {
-			ElasticsearchSimpleQueryStringPredicateBuilderFieldState field = fields.get( absoluteFieldPath );
-			if ( field == null ) {
-				field = scope.fieldQueryElement( absoluteFieldPath, ElasticsearchPredicateTypeKeys.SIMPLE_QUERY_STRING );
-				nestedCompatibilityChecker = nestedCompatibilityChecker.combineAndCheck( absoluteFieldPath );
-				fields.put( absoluteFieldPath, field );
+			ElasticsearchSimpleQueryStringPredicateBuilderFieldState fieldState = fieldStates.get( absoluteFieldPath );
+			if ( fieldState == null ) {
+				fieldState = scope.fieldQueryElement( absoluteFieldPath, ElasticsearchPredicateTypeKeys.SIMPLE_QUERY_STRING );
+				if ( firstFieldState == null ) {
+					firstFieldState = fieldState;
+				}
+				else {
+					SearchIndexSchemaElementContextHelper.checkNestedDocumentPathCompatibility(
+							firstFieldState.field(), fieldState.field() );
+				}
+				fieldStates.put( absoluteFieldPath, fieldState );
 			}
-			return field;
+			return fieldState;
 		}
 
 		@Override
@@ -199,7 +204,7 @@ public void skipAnalysis() {
 		@Override
 		public SearchPredicate build() {
 			if ( analyzer == null ) {
-				for ( ElasticsearchSimpleQueryStringPredicateBuilderFieldState field : fields.values() ) {
+				for ( ElasticsearchSimpleQueryStringPredicateBuilderFieldState field : fieldStates.values() ) {
 					field.checkAnalyzerOrNormalizerCompatibleAcrossIndexes();
 				}
 			}

diff --git a/...search/types/predicate/impl/ElasticsearchSimpleQueryStringPredicateBuilderFieldState.java b/...search/types/predicate/impl/ElasticsearchSimpleQueryStringPredicateBuilderFieldState.java
@@ -30,6 +30,10 @@ public void boost(float boost) {
 		this.boost = boost;
 	}
 
+	public ElasticsearchSearchIndexValueFieldContext<String> field() {
+		return field;
+	}
+
 	public void checkAnalyzerOrNormalizerCompatibleAcrossIndexes() {
 		field.type().searchAnalyzerName();
 		field.type().normalizerName();

diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java
@@ -468,13 +468,6 @@ SearchException unableToValidateIndexDirectory(String causeMessage,
 	SearchException unableToDropIndexDirectory(String causeMessage,
 			@Param EventContext context, @Cause Exception cause);
 
-	@Message(id = ID_OFFSET + 113,
-			value = "Invalid target fields for simple-query-string predicate:"
-					+ " fields [%1$s, %3$s] are in different nested documents [%2$s, %4$s]."
-					+ " All fields targeted by a simple-query-string predicate must be in the same document.")
-	SearchException simpleQueryStringSpanningMultipleNestedPaths(String fieldPath1, String nestedPath1,
-			String fieldPath2, String nestedPath2);
-
 	@Message(id = ID_OFFSET + 114,
 			value = "Invalid sort mode: %1$s. This sort mode is not supported for fields in nested documents.")
 	SearchException invalidSortModeAcrossNested(SortMode mode, @Param EventContext context);

diff --git a/...arch/backend/lucene/scope/model/impl/LuceneDifferentNestedObjectCompatibilityChecker.java b/...arch/backend/lucene/scope/model/impl/LuceneDifferentNestedObjectCompatibilityChecker.java
diff --git a/...bernate/search/backend/lucene/search/predicate/impl/LuceneSimpleQueryStringPredicate.java b/...bernate/search/backend/lucene/search/predicate/impl/LuceneSimpleQueryStringPredicate.java
@@ -18,11 +18,11 @@
 import org.hibernate.search.backend.lucene.analysis.model.impl.LuceneAnalysisDefinitionRegistry;
 import org.hibernate.search.backend.lucene.logging.impl.Log;
 import org.hibernate.search.backend.lucene.lowlevel.common.impl.AnalyzerConstants;
-import org.hibernate.search.backend.lucene.scope.model.impl.LuceneDifferentNestedObjectCompatibilityChecker;
 import org.hibernate.search.backend.lucene.search.common.impl.LuceneSearchIndexScope;
 import org.hibernate.search.backend.lucene.types.predicate.impl.LuceneSimpleQueryStringPredicateBuilderFieldState;
 import org.hibernate.search.engine.reporting.spi.EventContexts;
 import org.hibernate.search.engine.search.common.BooleanOperator;
+import org.hibernate.search.engine.search.common.spi.SearchIndexSchemaElementContextHelper;
 import org.hibernate.search.engine.search.predicate.SearchPredicate;
 import org.hibernate.search.engine.search.predicate.dsl.SimpleQueryFlag;
 import org.hibernate.search.engine.search.predicate.spi.SimpleQueryStringPredicateBuilder;
@@ -44,8 +44,8 @@ public class LuceneSimpleQueryStringPredicate extends AbstractLuceneNestablePred
 
 	private LuceneSimpleQueryStringPredicate(Builder builder) {
 		super( builder );
-		nestedPathHierarchy = builder.nestedCompatibilityChecker.getNestedPathHierarchy();
-		fieldPaths = new ArrayList<>( builder.fields.keySet() );
+		nestedPathHierarchy = builder.firstFieldState.field().nestedPathHierarchy();
+		fieldPaths = new ArrayList<>( builder.fieldStates.keySet() );
 		query = builder.buildQuery();
 	}
 
@@ -67,18 +67,17 @@ protected List<String> getFieldPathsForErrorMessage() {
 	public static class Builder extends AbstractBuilder implements SimpleQueryStringPredicateBuilder {
 		private final LuceneAnalysisDefinitionRegistry analysisDefinitionRegistry;
 
-		private final Map<String, LuceneSimpleQueryStringPredicateBuilderFieldState> fields = new LinkedHashMap<>();
+		private LuceneSimpleQueryStringPredicateBuilderFieldState firstFieldState;
+		private final Map<String, LuceneSimpleQueryStringPredicateBuilderFieldState> fieldStates = new LinkedHashMap<>();
 		private Occur defaultOperator = Occur.SHOULD;
 		private String simpleQueryString;
 		private Analyzer overrideAnalyzer;
 		private boolean ignoreAnalyzer = false;
 		private EnumSet<SimpleQueryFlag> flags;
-		private LuceneDifferentNestedObjectCompatibilityChecker nestedCompatibilityChecker;
 
 		Builder(LuceneSearchIndexScope scope) {
 			super( scope );
 			this.analysisDefinitionRegistry = scope.analysisDefinitionRegistry();
-			this.nestedCompatibilityChecker = LuceneDifferentNestedObjectCompatibilityChecker.empty( scope );
 		}
 
 		@Override
@@ -100,13 +99,18 @@ public void flags(Set<SimpleQueryFlag> flags) {
 
 		@Override
 		public FieldState field(String absoluteFieldPath) {
-			LuceneSimpleQueryStringPredicateBuilderFieldState field = fields.get( absoluteFieldPath );
-			if ( field == null ) {
-				field = scope.fieldQueryElement( absoluteFieldPath, LucenePredicateTypeKeys.SIMPLE_QUERY_STRING );
-				nestedCompatibilityChecker = nestedCompatibilityChecker.combineAndCheck( absoluteFieldPath );
-				fields.put( absoluteFieldPath, field );
+			LuceneSimpleQueryStringPredicateBuilderFieldState fieldState = fieldStates.get( absoluteFieldPath );
+			if ( fieldState == null ) {
+				fieldState = scope.fieldQueryElement( absoluteFieldPath, LucenePredicateTypeKeys.SIMPLE_QUERY_STRING );
+				if ( firstFieldState == null ) {
+					firstFieldState = fieldState;
+				}
+				else {
+					SearchIndexSchemaElementContextHelper.checkNestedDocumentPathCompatibility( firstFieldState.field(), fieldState.field() );
+				}
+				fieldStates.put( absoluteFieldPath, fieldState );
 			}
-			return field;
+			return fieldState;
 		}
 
 		@Override
@@ -145,8 +149,8 @@ private Analyzer buildAnalyzer() {
 			if ( overrideAnalyzer != null ) {
 				return overrideAnalyzer;
 			}
-			if ( fields.size() == 1 ) {
-				return fields.values().iterator().next().getAnalyzerOrNormalizer();
+			if ( fieldStates.size() == 1 ) {
+				return fieldStates.values().iterator().next().field().type().searchAnalyzerOrNormalizer();
 			}
 
 			/*
@@ -164,17 +168,17 @@ private Analyzer buildAnalyzer() {
 			 * would pick the first analyzer returned by any of the scoped analyzers in its list.
 			 */
 			ScopedAnalyzer.Builder builder = new ScopedAnalyzer.Builder();
-			for ( Map.Entry<String, LuceneSimpleQueryStringPredicateBuilderFieldState> entry : fields.entrySet() ) {
-				builder.setAnalyzer( entry.getKey(), entry.getValue().getAnalyzerOrNormalizer() );
+			for ( Map.Entry<String, LuceneSimpleQueryStringPredicateBuilderFieldState> entry : fieldStates.entrySet() ) {
+				builder.setAnalyzer( entry.getKey(), entry.getValue().field().type().searchAnalyzerOrNormalizer() );
 			}
 			return builder.build();
 		}
 
 		private Map<String, Float> buildWeights() {
 			Map<String, Float> weights = new LinkedHashMap<>();
-			for ( Map.Entry<String, LuceneSimpleQueryStringPredicateBuilderFieldState> entry : fields.entrySet() ) {
+			for ( Map.Entry<String, LuceneSimpleQueryStringPredicateBuilderFieldState> entry : fieldStates.entrySet() ) {
 				LuceneSimpleQueryStringPredicateBuilderFieldState state = entry.getValue();
-				Float boost = state.getBoost();
+				Float boost = state.boost();
 				if ( boost == null ) {
 					boost = 1f;
 				}

diff --git a/...ackend/lucene/types/predicate/impl/LuceneSimpleQueryStringPredicateBuilderFieldState.java b/...ackend/lucene/types/predicate/impl/LuceneSimpleQueryStringPredicateBuilderFieldState.java
@@ -11,8 +11,6 @@
 import org.hibernate.search.backend.lucene.search.common.impl.LuceneSearchIndexValueFieldContext;
 import org.hibernate.search.engine.search.predicate.spi.SimpleQueryStringPredicateBuilder;
 
-import org.apache.lucene.analysis.Analyzer;
-
 public final class LuceneSimpleQueryStringPredicateBuilderFieldState
 	implements SimpleQueryStringPredicateBuilder.FieldState {
 
@@ -28,11 +26,11 @@ public void boost(float boost) {
 		this.boost = boost;
 	}
 
-	public Analyzer getAnalyzerOrNormalizer() {
-		return field.type().searchAnalyzerOrNormalizer();
+	public LuceneSearchIndexValueFieldContext<?> field() {
+		return field;
 	}
 
-	public Float getBoost() {
+	public Float boost() {
 		return boost;
 	}
 

diff --git a/engine/src/main/java/org/hibernate/search/engine/logging/impl/Log.java b/engine/src/main/java/org/hibernate/search/engine/logging/impl/Log.java
@@ -501,4 +501,12 @@ SearchException inconsistentConfigurationForIndexElementForSearch(
 			value = "Unknown field '%1$s'.")
 	SearchException unknownFieldForSearch(String absoluteFieldPath, @Param EventContext context);
 
+	@Message(id = ID_OFFSET + 111,
+			value = "Invalid target fields:"
+					+ " fields [%1$s, %3$s] are in different nested documents (%2$s vs. %4$s)."
+					+ " All target fields must be in the same document.")
+	SearchException targetFieldsSpanningMultipleNestedPaths(String fieldPath1,
+			@FormatWith(EventContextNoPrefixFormatter.class) EventContext nestedPath1,
+			String fieldPath2, @FormatWith(EventContextNoPrefixFormatter.class) EventContext nestedPath2);
+
 }