From f1e4ba099dc99b0a0e528bbac1b81afd64ccec41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 20 Jul 2020 09:31:01 +0200 Subject: [PATCH] HSEARCH-3307 Simplify cross-index compatibility checks for text match predicates on Elasticsearch We have all the information we need in the field context, no need to add a "dataType" attribute to the factory. --- .../elasticsearch/logging/impl/Log.java | 5 ++-- ...ticsearchMultiIndexSearchFieldContext.java | 10 +++++++ .../ElasticsearchSearchFieldTypeContext.java | 3 +- ...searchStringIndexFieldTypeOptionsStep.java | 3 +- .../impl/ElasticsearchIndexFieldType.java | 5 ++++ .../impl/ElasticsearchTextMatchPredicate.java | 28 ++++--------------- .../ElasticsearchMatchSearchPredicateIT.java | 5 ++-- 7 files changed, 28 insertions(+), 31 deletions(-) diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java index e8101f7d27c..0b1c96794d1 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java @@ -412,8 +412,9 @@ SearchException conflictingIdentifierTypesForSearch(ToDocumentIdentifierValueCon SearchException unexpectedElasticsearchVersion(ElasticsearchVersion configuredVersion, ElasticsearchVersion actualVersion); - @Message(id = ID_OFFSET_3 + 60, value = "Elasticsearch backend does not support skip analysis on not analyzed field: '%1$s'.") - SearchException skipAnalysisOnKeywordField(String absoluteFieldPath, @Param EventContext context); + @Message(id = ID_OFFSET_3 + 60, value = "Cannot skip analysis on field '%1$s':" + + " the Elasticsearch backend will always normalize arguments before attempting matches on normalized fields.") + SearchException skipAnalysisOnNormalizedField(String absoluteFieldPath, @Param EventContext context); @Message(id = ID_OFFSET_3 + 61, value = "Ambiguous Elasticsearch version: '%s'." diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchMultiIndexSearchFieldContext.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchMultiIndexSearchFieldContext.java index 2d5c647a346..4b1a0054ecf 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchMultiIndexSearchFieldContext.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchMultiIndexSearchFieldContext.java @@ -123,6 +123,16 @@ public Optional normalizerName() { "normalizer" ); } + @Override + public boolean hasNormalizerOnAtLeastOneIndex() { + for ( ElasticsearchSearchFieldContext fieldContext : fieldForEachIndex ) { + if ( fieldContext.type().hasNormalizerOnAtLeastOneIndex() ) { + return true; + } + } + return false; + } + @Override public ElasticsearchSearchFieldQueryElementFactory queryElementFactory(SearchQueryElementTypeKey key) { ElasticsearchSearchFieldQueryElementFactory factory = null; diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchSearchFieldTypeContext.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchSearchFieldTypeContext.java index ab6518f83ee..897885d1c7f 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchSearchFieldTypeContext.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/impl/ElasticsearchSearchFieldTypeContext.java @@ -52,6 +52,7 @@ default DslConverter dslConverter(ValueConvert convert) { Optional normalizerName(); - ElasticsearchSearchFieldQueryElementFactory queryElementFactory(SearchQueryElementTypeKey key); + boolean hasNormalizerOnAtLeastOneIndex(); + ElasticsearchSearchFieldQueryElementFactory queryElementFactory(SearchQueryElementTypeKey key); } diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/impl/ElasticsearchStringIndexFieldTypeOptionsStep.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/impl/ElasticsearchStringIndexFieldTypeOptionsStep.java index bf330feae0c..2b5d7d597d4 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/impl/ElasticsearchStringIndexFieldTypeOptionsStep.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/dsl/impl/ElasticsearchStringIndexFieldTypeOptionsStep.java @@ -180,8 +180,7 @@ public IndexFieldType toIndexFieldType() { if ( resolvedSearchable ) { builder.searchable( true ); - builder.queryElementFactory( PredicateTypeKeys.MATCH, - new ElasticsearchTextMatchPredicate.Factory( codec, mapping.getType() ) ); + builder.queryElementFactory( PredicateTypeKeys.MATCH, new ElasticsearchTextMatchPredicate.Factory( codec ) ); builder.queryElementFactory( PredicateTypeKeys.RANGE, new ElasticsearchRangePredicate.Factory<>( codec ) ); builder.queryElementFactory( PredicateTypeKeys.EXISTS, new ElasticsearchExistsPredicate.Factory<>() ); builder.queryElementFactory( PredicateTypeKeys.PHRASE, new ElasticsearchTextPhrasePredicate.Factory() ); diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/impl/ElasticsearchIndexFieldType.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/impl/ElasticsearchIndexFieldType.java index ad129921d18..f2a40a2cd03 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/impl/ElasticsearchIndexFieldType.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/impl/ElasticsearchIndexFieldType.java @@ -142,6 +142,11 @@ public Optional normalizerName() { return Optional.ofNullable( normalizerName ); } + @Override + public boolean hasNormalizerOnAtLeastOneIndex() { + return normalizerName().isPresent(); + } + @Override public Optional searchAnalyzerName() { return Optional.ofNullable( searchAnalyzerName ); diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/predicate/impl/ElasticsearchTextMatchPredicate.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/predicate/impl/ElasticsearchTextMatchPredicate.java index f62ac752c3c..f0b87ef0563 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/predicate/impl/ElasticsearchTextMatchPredicate.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/predicate/impl/ElasticsearchTextMatchPredicate.java @@ -11,11 +11,9 @@ import org.hibernate.search.backend.elasticsearch.gson.impl.JsonAccessor; import org.hibernate.search.backend.elasticsearch.logging.impl.Log; import org.hibernate.search.backend.elasticsearch.lowlevel.index.analysis.impl.AnalyzerConstants; -import org.hibernate.search.backend.elasticsearch.lowlevel.index.mapping.impl.DataTypes; import org.hibernate.search.backend.elasticsearch.search.impl.AbstractElasticsearchCodecAwareSearchFieldQueryElementFactory; import org.hibernate.search.backend.elasticsearch.search.impl.ElasticsearchSearchContext; import org.hibernate.search.backend.elasticsearch.search.impl.ElasticsearchSearchFieldContext; -import org.hibernate.search.backend.elasticsearch.search.impl.ElasticsearchSearchFieldQueryElementFactory; import org.hibernate.search.backend.elasticsearch.search.predicate.impl.PredicateRequestContext; import org.hibernate.search.backend.elasticsearch.types.codec.impl.ElasticsearchFieldCodec; import org.hibernate.search.engine.reporting.spi.EventContexts; @@ -61,41 +59,25 @@ protected JsonObject doToJsonQuery(PredicateRequestContext context, JsonObject o public static class Factory extends AbstractElasticsearchCodecAwareSearchFieldQueryElementFactory { - private final String dataType; - - public Factory(ElasticsearchFieldCodec codec, String dataType) { + public Factory(ElasticsearchFieldCodec codec) { super( codec ); - this.dataType = dataType; - } - - @Override - public boolean isCompatibleWith(ElasticsearchSearchFieldQueryElementFactory other) { - if ( !super.isCompatibleWith( other ) ) { - return false; - } - - Factory castedOther = (Factory) other; - return dataType.equals( castedOther.dataType ); } @Override public MatchPredicateBuilder create(ElasticsearchSearchContext searchContext, ElasticsearchSearchFieldContext field) { - return new Builder( codec, dataType, searchContext, field ); + return new Builder( codec, searchContext, field ); } } private static class Builder extends ElasticsearchStandardMatchPredicate.Builder { - private final String type; - private Integer fuzziness; private Integer prefixLength; private String analyzer; - private Builder(ElasticsearchFieldCodec codec, String type, ElasticsearchSearchContext searchContext, + private Builder(ElasticsearchFieldCodec codec, ElasticsearchSearchContext searchContext, ElasticsearchSearchFieldContext field) { super( codec, searchContext, field ); - this.type = type; } @Override @@ -111,8 +93,8 @@ public void analyzer(String analyzerName) { @Override public void skipAnalysis() { - if ( DataTypes.KEYWORD.equals( type ) ) { - throw log.skipAnalysisOnKeywordField( absoluteFieldPath, + if ( field.type().hasNormalizerOnAtLeastOneIndex() ) { + throw log.skipAnalysisOnNormalizedField( absoluteFieldPath, EventContexts.fromIndexFieldAbsolutePath( absoluteFieldPath ) ); } diff --git a/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/search/ElasticsearchMatchSearchPredicateIT.java b/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/search/ElasticsearchMatchSearchPredicateIT.java index 4ff957c2ed1..83ec36b69be 100644 --- a/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/search/ElasticsearchMatchSearchPredicateIT.java +++ b/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/search/ElasticsearchMatchSearchPredicateIT.java @@ -41,9 +41,8 @@ public void match_skipAnalysis_normalizedStringField() { .toQuery() ) .isInstanceOf( SearchException.class ) - .hasMessageContaining( "HSEARCH400560" ) - .hasMessageContaining( "Elasticsearch backend does not support skip analysis on not analyzed field" ) - .hasMessageContaining( "normalizedStringField" ); + .hasMessageContainingAll( "Cannot skip analysis on field 'normalizedStringField'", + "the Elasticsearch backend will always normalize arguments before attempting matches on normalized fields" ); } private void initData() {