From 2e36d0ea1c64b7a54c50dd4dc2d151d15786f73c Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Thu, 6 Jun 2024 09:46:06 +0200 Subject: [PATCH] HSEARCH-5164 Introduce a new dialect to always pass the index_options.type=hnsw --- .../impl/ElasticsearchDialectFactory.java | 7 ++++- .../impl/Elasticsearch814ModelDialect.java | 30 +++++++++++++++++++ ...earch812IndexFieldTypeFactoryProvider.java | 2 +- ...earch814IndexFieldTypeFactoryProvider.java | 30 +++++++++++++++++++ ...h812VectorFieldTypeMappingContributor.java | 6 +++- ...h814VectorFieldTypeMappingContributor.java | 16 ++++++++++ ...ch814PropertyMappingValidatorProvider.java | 16 ++++++++++ .../impl/PropertyMappingValidator.java | 29 ++++++++++++++++-- .../impl/ElasticsearchDialectFactoryTest.java | 11 +++---- .../dialect/ElasticsearchTestDialect.java | 26 ++++++++++------ 10 files changed, 153 insertions(+), 20 deletions(-) create mode 100644 backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/model/impl/Elasticsearch814ModelDialect.java create mode 100644 backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch814IndexFieldTypeFactoryProvider.java create mode 100644 backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch814VectorFieldTypeMappingContributor.java create mode 100644 backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/Elasticsearch814PropertyMappingValidatorProvider.java diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactory.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactory.java index a9a851ae5c6..af88428ae33 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactory.java +++ b/backend/elasticsearch/src/main/java/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/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/model/impl/Elasticsearch814ModelDialect.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/dialect/model/impl/Elasticsearch814ModelDialect.java new file mode 100644 index 00000000000..60fa1d0b48b --- /dev/null +++ b/backend/elasticsearch/src/main/java/org/hibernate/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 . + */ +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/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch812IndexFieldTypeFactoryProvider.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch812IndexFieldTypeFactoryProvider.java index 80f6c06941f..a9bb5b2aa7f 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch812IndexFieldTypeFactoryProvider.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/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/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch814IndexFieldTypeFactoryProvider.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/provider/impl/Elasticsearch814IndexFieldTypeFactoryProvider.java new file mode 100644 index 00000000000..9b7d39b6d1f --- /dev/null +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/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 . + */ +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/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch812VectorFieldTypeMappingContributor.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch812VectorFieldTypeMappingContributor.java index ccd6c7e1a3a..8b0c361e279 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch812VectorFieldTypeMappingContributor.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/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 void contribute(ElasticsearchIndexValueFieldType.Builder builder, Context context) { builder.queryElementFactory( PredicateTypeKeys.KNN, diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch814VectorFieldTypeMappingContributor.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/mapping/impl/Elasticsearch814VectorFieldTypeMappingContributor.java new file mode 100644 index 00000000000..b56ede5e623 --- /dev/null +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/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 . + */ +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/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/Elasticsearch814PropertyMappingValidatorProvider.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/Elasticsearch814PropertyMappingValidatorProvider.java new file mode 100644 index 00000000000..f2620a0d663 --- /dev/null +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/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 . + */ +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 create() { + return new PropertyMappingValidator.Elasticsearch814PropertyMappingValidator(); + } +} diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/PropertyMappingValidator.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/PropertyMappingValidator.java index 45f4532cc79..c648582e622 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/validation/impl/PropertyMappingValidator.java +++ b/backend/elasticsearch/src/main/java/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/backend/elasticsearch/src/test/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactoryTest.java b/backend/elasticsearch/src/test/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactoryTest.java index 986b1776006..cd98def03fd 100644 --- a/backend/elasticsearch/src/test/java/org/hibernate/search/backend/elasticsearch/dialect/impl/ElasticsearchDialectFactoryTest.java +++ b/backend/elasticsearch/src/test/java/org/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 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/util/internal/integrationtest/backend/elasticsearch/src/main/java/org/hibernate/search/util/impl/integrationtest/backend/elasticsearch/dialect/ElasticsearchTestDialect.java b/util/internal/integrationtest/backend/elasticsearch/src/main/java/org/hibernate/search/util/impl/integrationtest/backend/elasticsearch/dialect/ElasticsearchTestDialect.java index ece5cc67790..2322285cb48 100644 --- a/util/internal/integrationtest/backend/elasticsearch/src/main/java/org/hibernate/search/util/impl/integrationtest/backend/elasticsearch/dialect/ElasticsearchTestDialect.java +++ b/util/internal/integrationtest/backend/elasticsearch/src/main/java/org/hibernate/search/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: