HSEARCH-5164 Introduce a new dialect to always pass the index_options…

….type=hnsw
hibernate · Jun 7, 2024 · 2e36d0e · 2e36d0e
1 parent ad88854
commit 2e36d0e
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 20 deletions.
diff --git a/.../org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactory.java b/.../org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactory.java
@@ -13,6 +13,7 @@
 import org.hibernate.search.backend.elasticsearch.ElasticsearchVersion;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch7ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch812ModelDialect;
+import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch814ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch8ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.ElasticsearchModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.OpenSearch1ModelDialect;
@@ -104,7 +105,11 @@ else if ( major == 7 ) {
 			if ( major == 8 && ( minorOptional.isEmpty() || minorOptional.getAsInt() < 12 ) ) {
 				return new Elasticsearch8ModelDialect();
 			}
-			return new Elasticsearch812ModelDialect();
+			if ( major == 8 && minorOptional.getAsInt() < 14 ) {
+				return new Elasticsearch812ModelDialect();
+			}
+
+			return new Elasticsearch814ModelDialect();
 		}
 	}
 

diff --git a/...bernate/search/backend/elasticsearch/dialect/model/impl/Elasticsearch814ModelDialect.java b/...bernate/search/backend/elasticsearch/dialect/model/impl/Elasticsearch814ModelDialect.java
@@ -0,0 +1,30 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.backend.elasticsearch.dialect.model.impl;
+
+import org.hibernate.search.backend.elasticsearch.types.dsl.provider.impl.Elasticsearch814IndexFieldTypeFactoryProvider;
+import org.hibernate.search.backend.elasticsearch.types.dsl.provider.impl.ElasticsearchIndexFieldTypeFactoryProvider;
+import org.hibernate.search.backend.elasticsearch.validation.impl.Elasticsearch814PropertyMappingValidatorProvider;
+import org.hibernate.search.backend.elasticsearch.validation.impl.ElasticsearchPropertyMappingValidatorProvider;
+
+import com.google.gson.Gson;
+
+/**
+ * The model dialect for Elasticsearch 8.14+.
+ */
+public class Elasticsearch814ModelDialect implements ElasticsearchModelDialect {
+
+	@Override
+	public ElasticsearchIndexFieldTypeFactoryProvider createIndexTypeFieldFactoryProvider(Gson userFacingGson) {
+		return new Elasticsearch814IndexFieldTypeFactoryProvider( userFacingGson );
+	}
+
+	@Override
+	public ElasticsearchPropertyMappingValidatorProvider createElasticsearchPropertyMappingValidatorProvider() {
+		return new Elasticsearch814PropertyMappingValidatorProvider();
+	}
+}
diff --git a/.../elasticsearch/types/dsl/provider/impl/Elasticsearch812IndexFieldTypeFactoryProvider.java b/.../elasticsearch/types/dsl/provider/impl/Elasticsearch812IndexFieldTypeFactoryProvider.java
@@ -12,7 +12,7 @@
 import com.google.gson.Gson;
 
 /**
- * The index field type factory provider for ES8.12+.
+ * The index field type factory provider for ES8.12-8.13.
  */
 public class Elasticsearch812IndexFieldTypeFactoryProvider extends AbstractIndexFieldTypeFactoryProvider {
 

diff --git a/.../elasticsearch/types/dsl/provider/impl/Elasticsearch814IndexFieldTypeFactoryProvider.java b/.../elasticsearch/types/dsl/provider/impl/Elasticsearch814IndexFieldTypeFactoryProvider.java
@@ -0,0 +1,30 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.backend.elasticsearch.types.dsl.provider.impl;
+
+import org.hibernate.search.backend.elasticsearch.types.mapping.impl.Elasticsearch814VectorFieldTypeMappingContributor;
+import org.hibernate.search.backend.elasticsearch.types.mapping.impl.ElasticsearchVectorFieldTypeMappingContributor;
+
+import com.google.gson.Gson;
+
+/**
+ * The index field type factory provider for ES8.14+.
+ */
+public class Elasticsearch814IndexFieldTypeFactoryProvider extends AbstractIndexFieldTypeFactoryProvider {
+
+	private final Elasticsearch814VectorFieldTypeMappingContributor vectorFieldTypeMappingContributor =
+			new Elasticsearch814VectorFieldTypeMappingContributor();
+
+	public Elasticsearch814IndexFieldTypeFactoryProvider(Gson userFacingGson) {
+		super( userFacingGson );
+	}
+
+	@Override
+	protected ElasticsearchVectorFieldTypeMappingContributor vectorFieldTypeMappingContributor() {
+		return vectorFieldTypeMappingContributor;
+	}
+}
diff --git a/...d/elasticsearch/types/mapping/impl/Elasticsearch812VectorFieldTypeMappingContributor.java b/...d/elasticsearch/types/mapping/impl/Elasticsearch812VectorFieldTypeMappingContributor.java
@@ -26,7 +26,7 @@ public void contribute(PropertyMapping mapping, Context context) {
 		if ( resolvedVectorSimilarity != null ) {
 			mapping.setSimilarity( resolvedVectorSimilarity );
 		}
-		if ( context.m() != null || context.efConstruction() != null ) {
+		if ( indexOptionAddCondition( context ) ) {
 			ElasticsearchDenseVectorIndexOptions indexOptions = new ElasticsearchDenseVectorIndexOptions();
 			indexOptions.setType( "hnsw" );
 			if ( context.m() != null ) {
@@ -39,6 +39,10 @@ public void contribute(PropertyMapping mapping, Context context) {
 		}
 	}
 
+	protected boolean indexOptionAddCondition(Context context) {
+		return context.m() != null || context.efConstruction() != null;
+	}
+
 	@Override
 	public <F> void contribute(ElasticsearchIndexValueFieldType.Builder<F> builder, Context context) {
 		builder.queryElementFactory( PredicateTypeKeys.KNN,

diff --git a/...d/elasticsearch/types/mapping/impl/Elasticsearch814VectorFieldTypeMappingContributor.java b/...d/elasticsearch/types/mapping/impl/Elasticsearch814VectorFieldTypeMappingContributor.java
@@ -0,0 +1,16 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.backend.elasticsearch.types.mapping.impl;
+
+public class Elasticsearch814VectorFieldTypeMappingContributor extends Elasticsearch812VectorFieldTypeMappingContributor {
+
+	@Override
+	protected boolean indexOptionAddCondition(Context context) {
+		// we want to always add index options and in particular include `hnsw` type.
+		return context.searchable();
+	}
+}
diff --git a/...ckend/elasticsearch/validation/impl/Elasticsearch814PropertyMappingValidatorProvider.java b/...ckend/elasticsearch/validation/impl/Elasticsearch814PropertyMappingValidatorProvider.java
@@ -0,0 +1,16 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.backend.elasticsearch.validation.impl;
+
+import org.hibernate.search.backend.elasticsearch.lowlevel.index.mapping.impl.PropertyMapping;
+
+public class Elasticsearch814PropertyMappingValidatorProvider implements ElasticsearchPropertyMappingValidatorProvider {
+	@Override
+	public Validator<PropertyMapping> create() {
+		return new PropertyMappingValidator.Elasticsearch814PropertyMappingValidator();
+	}
+}
diff --git a/.../org/hibernate/search/backend/elasticsearch/validation/impl/PropertyMappingValidator.java b/.../org/hibernate/search/backend/elasticsearch/validation/impl/PropertyMappingValidator.java
@@ -136,7 +136,24 @@ protected void validateVectorMapping(ValidationErrorCollector errorCollector, Pr
 		}
 	}
 
-	static class Elasticsearch812PropertyMappingValidator extends PropertyMappingValidator {
+	static class Elasticsearch812PropertyMappingValidator extends Elasticsearch8xPropertyMappingValidator {
+	}
+
+	static class Elasticsearch814PropertyMappingValidator extends Elasticsearch8xPropertyMappingValidator {
+		@Override
+		protected boolean indexOptionsRequireValidation(ElasticsearchDenseVectorIndexOptions expected,
+				ElasticsearchDenseVectorIndexOptions actual) {
+			if ( expected != null
+					&& actual == null
+					&& expected.getEfConstruction() == null && expected.getM() == null && expected.getType() != null ) {
+				// if we set type only then ES will not return the index option block ... so we skip
+				return false;
+			}
+			return super.indexOptionsRequireValidation( expected, actual );
+		}
+	}
+
+	static class Elasticsearch8xPropertyMappingValidator extends PropertyMappingValidator {
 
 		private final ElasticsearchDenseVectorIndexOptionsValidator indexOptionsValidator =
 				new ElasticsearchDenseVectorIndexOptionsValidator();
@@ -160,10 +177,16 @@ protected void validateVectorMapping(ValidationErrorCollector errorCollector, Pr
 			);
 
 			ElasticsearchDenseVectorIndexOptions indexOptions = expectedMapping.getIndexOptions();
-			if ( indexOptions != null ) {
-				indexOptionsValidator.validate( errorCollector, indexOptions, actualMapping.getIndexOptions() );
+			ElasticsearchDenseVectorIndexOptions actual = actualMapping.getIndexOptions();
+			if ( indexOptionsRequireValidation( indexOptions, actual ) ) {
+				indexOptionsValidator.validate( errorCollector, indexOptions, actual );
 			}
 		}
+
+		protected boolean indexOptionsRequireValidation(ElasticsearchDenseVectorIndexOptions expected,
+				ElasticsearchDenseVectorIndexOptions actual) {
+			return expected != null;
+		}
 	}
 
 	static class OpenSearch1PropertyMappingValidator extends PropertyMappingValidator {

diff --git a/.../hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactoryTest.java b/.../hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactoryTest.java
@@ -16,6 +16,7 @@
 import org.hibernate.search.backend.elasticsearch.ElasticsearchVersion;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch7ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch812ModelDialect;
+import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch814ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.Elasticsearch8ModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.ElasticsearchModelDialect;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.OpenSearch1ModelDialect;
@@ -259,23 +260,23 @@ public static List<? extends Arguments> params() {
 				),
 				success(
 						ElasticsearchDistributionName.ELASTIC, "8.14", "8.14.0",
-						Elasticsearch812ModelDialect.class, Elasticsearch81ProtocolDialect.class
+						Elasticsearch814ModelDialect.class, Elasticsearch81ProtocolDialect.class
 				),
 				success(
 						ElasticsearchDistributionName.ELASTIC, "8.14.0", "8.14.0",
-						Elasticsearch812ModelDialect.class, Elasticsearch81ProtocolDialect.class
+						Elasticsearch814ModelDialect.class, Elasticsearch81ProtocolDialect.class
 				),
 				successWithWarning(
 						ElasticsearchDistributionName.ELASTIC, "8.15", "8.15.0",
-						Elasticsearch812ModelDialect.class, Elasticsearch81ProtocolDialect.class
+						Elasticsearch814ModelDialect.class, Elasticsearch81ProtocolDialect.class
 				),
 				successWithWarning(
 						ElasticsearchDistributionName.ELASTIC, "8.15.0", "8.15.0",
-						Elasticsearch812ModelDialect.class, Elasticsearch81ProtocolDialect.class
+						Elasticsearch814ModelDialect.class, Elasticsearch81ProtocolDialect.class
 				),
 				successWithWarning(
 						ElasticsearchDistributionName.ELASTIC, "9.0.0", "9.0.0",
-						Elasticsearch812ModelDialect.class, Elasticsearch81ProtocolDialect.class
+						Elasticsearch814ModelDialect.class, Elasticsearch81ProtocolDialect.class
 				),
 				success(
 						ElasticsearchDistributionName.OPENSEARCH, "1", "1.3.1",

diff --git a/...rch/util/impl/integrationtest/backend/elasticsearch/dialect/ElasticsearchTestDialect.java b/...rch/util/impl/integrationtest/backend/elasticsearch/dialect/ElasticsearchTestDialect.java
@@ -82,15 +82,23 @@ public String vectorFieldMapping(int dim, Class<?> elementType, OptionalInt m, O
 						+ "  'element_type': '" + elementType.getName() + "',"
 						+ "  'dims': " + dim
 						+ similarity.map( s -> ",  'similarity': '" + s + "'" ).orElse( "" )
-						+ ( ( m.isPresent() || efConstruction.isPresent() )
-								? ( ",  'index_options': {"
-										+ ( m.isPresent() ? "    'm': " + m.getAsInt() + "," : "" )
-										+ ( efConstruction.isPresent()
-												? "    'ef_construction': " + efConstruction.getAsInt() + ","
-												: "" )
-										+ "    'type': 'hnsw'"
-										+ "  }" )
-								: "" )
+						+ ( ( m.isPresent()
+								|| efConstruction.isPresent()
+								|| !isActualVersion(
+										esVersion -> esVersion.isLessThan( "8.14.0" ),
+										osVersion -> {
+											throw new AssertionFailure(
+													"OpenSearch cannot be called within this condition block" );
+										}
+								) )
+										? ( ",  'index_options': {"
+												+ ( m.isPresent() ? "    'm': " + m.getAsInt() + "," : "" )
+												+ ( efConstruction.isPresent()
+														? "    'ef_construction': " + efConstruction.getAsInt() + ","
+														: "" )
+												+ "    'type': 'hnsw'"
+												+ "  }" )
+										: "" )
 						+ "}";
 			case OPENSEARCH:
 			case AMAZON_OPENSEARCH_SERVERLESS: