diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchDistanceSort.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchDistanceSort.java index 10f3c50e163..df65ef13691 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchDistanceSort.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchDistanceSort.java @@ -41,11 +41,11 @@ private ElasticsearchDistanceSort(Builder builder) { @Override protected void doToJsonSorts(ElasticsearchSearchSortCollector collector, JsonObject innerObject) { innerObject.add( absoluteFieldPath, ElasticsearchGeoPointFieldCodec.INSTANCE.encode( center ) ); - if ( indexNames.size() > 1 ) { - // There are multiple target indexes; some of them may not declare the field. - // Instruct ES to behave as if the field had no value in that case. - searchSyntax.requestGeoDistanceSortIgnoreUnmapped( innerObject ); - } + // If there are multiple target indexes, or if the field is dynamic, + // some target indexes may not have this field in their mapping (yet), + // and in that case Elasticsearch would raise an exception. + // Instruct ES to behave as if the field had no value in that case. + searchSyntax.requestGeoDistanceSortIgnoreUnmapped( innerObject ); JsonObject outerObject = new JsonObject(); GEO_DISTANCE_ACCESSOR.set( outerObject, innerObject ); diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchStandardFieldSort.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchStandardFieldSort.java index f00a001c65e..42786d9ccf0 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchStandardFieldSort.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/types/sort/impl/ElasticsearchStandardFieldSort.java @@ -43,12 +43,7 @@ public class ElasticsearchStandardFieldSort extends AbstractElasticsearchDocumen private ElasticsearchStandardFieldSort(Builder builder) { super( builder ); missing = builder.missing; - if ( indexNames().size() > 1 ) { - unmappedType = builder.field.type().elasticsearchTypeAsJson(); - } - else { - unmappedType = null; - } + unmappedType = builder.field.type().elasticsearchTypeAsJson(); } @Override @@ -61,7 +56,9 @@ public void doToJsonSorts(ElasticsearchSearchSortCollector collector, JsonObject // Elasticsearch complains it needs a scaling factor, but we don't have any way to provide it. // See https://hibernate.atlassian.net/browse/HSEARCH-4176 if ( unmappedType != null && !DataTypes.SCALED_FLOAT.equals( unmappedType.getAsString() ) ) { - // There are multiple target indexes; some of them may not declare the field. + // If there are multiple target indexes, or if the field is dynamic, + // some target indexes may not have this field in their mapping (yet), + // and in that case Elasticsearch would raise an exception. // Instruct ES to behave as if the field had no value in that case. UNMAPPED_TYPE.set( innerObject, unmappedType ); } diff --git a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/DistanceSortDynamicFieldIT.java b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/DistanceSortDynamicFieldIT.java new file mode 100644 index 00000000000..da46ab378c8 --- /dev/null +++ b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/DistanceSortDynamicFieldIT.java @@ -0,0 +1,146 @@ +/* + * 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.integrationtest.backend.tck.search.sort; + +import static org.hibernate.search.integrationtest.backend.tck.testsupport.types.values.AscendingUniqueDistanceFromCenterValues.CENTER_POINT; +import static org.hibernate.search.util.impl.integrationtest.common.assertion.SearchResultAssert.assertThatQuery; + +import java.util.function.Function; + +import org.hibernate.search.engine.backend.common.DocumentReference; +import org.hibernate.search.engine.backend.document.DocumentElement; +import org.hibernate.search.engine.backend.document.model.dsl.IndexSchemaElement; +import org.hibernate.search.engine.backend.types.Sortable; +import org.hibernate.search.engine.search.query.SearchQuery; +import org.hibernate.search.engine.search.sort.dsl.SearchSortFactory; +import org.hibernate.search.engine.search.sort.dsl.SortFinalStep; +import org.hibernate.search.integrationtest.backend.tck.testsupport.types.GeoPointFieldTypeDescriptor; +import org.hibernate.search.integrationtest.backend.tck.testsupport.types.values.AscendingUniqueDistanceFromCenterValues; +import org.hibernate.search.integrationtest.backend.tck.testsupport.util.rule.SearchSetupHelper; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.BulkIndexer; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.SimpleMappedIndex; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.StubMappingScope; +import org.hibernate.search.util.impl.test.annotation.TestForIssue; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +public class DistanceSortDynamicFieldIT { + + private static final GeoPointFieldTypeDescriptor fieldType = GeoPointFieldTypeDescriptor.INSTANCE; + + private static final String DOCUMENT_1 = "1"; + private static final String DOCUMENT_2 = "2"; + private static final String DOCUMENT_3 = "3"; + + private static final String EMPTY = "empty"; + + private static final int DOCUMENT_1_ORDINAL = 1; + private static final int DOCUMENT_2_ORDINAL = 3; + private static final int DOCUMENT_3_ORDINAL = 5; + + @ClassRule + public static SearchSetupHelper setupHelper = new SearchSetupHelper(); + + private static final SimpleMappedIndex mainIndex = + SimpleMappedIndex.of( IndexBinding::new ).name( "main" ); + + @BeforeClass + public static void setup() { + setupHelper.start() + .withIndexes( mainIndex ) + .setup(); + + initData(); + } + + @Test + public void simple() { + String fieldPath = mainFieldPath(); + + assertThatQuery( matchNonEmptyQuery( f -> f.distance( fieldPath, CENTER_POINT ).asc() ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_1, DOCUMENT_2, DOCUMENT_3 ); + assertThatQuery( matchNonEmptyQuery( f -> f.distance( fieldPath, CENTER_POINT ).desc() ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_3, DOCUMENT_2, DOCUMENT_1 ); + } + + @Test + @TestForIssue(jiraKey = "HSEARCH-4531") + public void neverPopulated() { + String neverPopulatedFieldPath = neverPopulatedFieldPath(); + String mainFieldPath = mainFieldPath(); + + // The field that wasn't populated shouldn't have any effect on the sort, + // but it shouldn't trigger an exception, either (see HSEARCH-4531). + assertThatQuery( matchNonEmptyQuery( f -> f.composite() + .add( f.distance( neverPopulatedFieldPath, CENTER_POINT ).asc() ) + .add( f.distance( mainFieldPath, CENTER_POINT ).asc() ) ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_1, DOCUMENT_2, DOCUMENT_3 ); + assertThatQuery( matchNonEmptyQuery( f -> f.composite() + .add( f.distance( neverPopulatedFieldPath, CENTER_POINT ).desc() ) + .add( f.distance( mainFieldPath, CENTER_POINT ).desc() ) ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_3, DOCUMENT_2, DOCUMENT_1 ); + } + + private SearchQuery matchNonEmptyQuery( + Function sortContributor) { + return matchNonEmptyQuery( sortContributor, mainIndex.createScope() ); + } + + private SearchQuery matchNonEmptyQuery( + Function sortContributor, StubMappingScope scope) { + return scope.query() + .where( f -> f.matchAll().except( f.id().matching( EMPTY ) ) ) + .sort( sortContributor ) + .toQuery(); + } + + private static String mainFieldPath() { + return IndexBinding.fieldPath( "main" ); + } + + private static String neverPopulatedFieldPath() { + return IndexBinding.fieldPath( "neverPopulated" ); + } + + private static void initData() { + BulkIndexer mainIndexer = mainIndex.bulkIndexer() + // Important: do not index the documents in the expected order after sorts (1, 2, 3) + .add( DOCUMENT_2, document -> initDocument( document, DOCUMENT_2_ORDINAL ) ) + .add( EMPTY, document -> initDocument( document, null ) ) + .add( DOCUMENT_1, document -> initDocument( document, DOCUMENT_1_ORDINAL ) ) + .add( DOCUMENT_3, document -> initDocument( document, DOCUMENT_3_ORDINAL ) ); + mainIndexer.join(); + } + + private static void initDocument(DocumentElement document, Integer ordinal) { + addValue( document, ordinal ); + } + + private static void addValue(DocumentElement documentElement, Integer ordinal) { + if ( ordinal == null ) { + return; + } + documentElement.addValue( + mainFieldPath(), + AscendingUniqueDistanceFromCenterValues.INSTANCE.getSingle().get( ordinal ) + ); + } + + private static class IndexBinding { + public static String fieldPath(String suffix) { + return "geoPoint_" + suffix; + } + + IndexBinding(IndexSchemaElement root) { + root.fieldTemplate( "myTemplate", f -> fieldType.configure( f ).sortable( Sortable.YES ) ) + .matchingPathGlob( fieldPath( "*" ) ); + } + } + +} diff --git a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/FieldSortDynamicFieldIT.java b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/FieldSortDynamicFieldIT.java new file mode 100644 index 00000000000..18e60fe4cbc --- /dev/null +++ b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/FieldSortDynamicFieldIT.java @@ -0,0 +1,185 @@ +/* + * 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.integrationtest.backend.tck.search.sort; + +import static org.hibernate.search.util.impl.integrationtest.common.assertion.SearchResultAssert.assertThatQuery; +import static org.junit.Assume.assumeTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Function; + +import org.hibernate.search.engine.backend.common.DocumentReference; +import org.hibernate.search.engine.backend.document.DocumentElement; +import org.hibernate.search.engine.backend.document.model.dsl.IndexSchemaElement; +import org.hibernate.search.engine.backend.types.Sortable; +import org.hibernate.search.engine.search.query.SearchQuery; +import org.hibernate.search.engine.search.sort.dsl.SearchSortFactory; +import org.hibernate.search.engine.search.sort.dsl.SortFinalStep; +import org.hibernate.search.integrationtest.backend.tck.testsupport.types.FieldTypeDescriptor; +import org.hibernate.search.integrationtest.backend.tck.testsupport.util.TckConfiguration; +import org.hibernate.search.integrationtest.backend.tck.testsupport.util.rule.SearchSetupHelper; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.BulkIndexer; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.SimpleMappedIndex; +import org.hibernate.search.util.impl.integrationtest.mapper.stub.StubMappingScope; +import org.hibernate.search.util.impl.test.annotation.TestForIssue; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class FieldSortDynamicFieldIT { + + private static final List> supportedFieldTypes = new ArrayList<>(); + + @Parameterized.Parameters(name = "{0}") + public static Object[][] parameters() { + List parameters = new ArrayList<>(); + for ( FieldTypeDescriptor fieldType : FieldTypeDescriptor.getAll() ) { + if ( fieldType.isFieldSortSupported() ) { + supportedFieldTypes.add( fieldType ); + parameters.add( new Object[] { fieldType } ); + } + } + return parameters.toArray( new Object[0][] ); + } + + private static final String DOCUMENT_1 = "1"; + private static final String DOCUMENT_2 = "2"; + private static final String DOCUMENT_3 = "3"; + + private static final String EMPTY = "empty"; + + private static final int DOCUMENT_1_ORDINAL = 1; + private static final int DOCUMENT_2_ORDINAL = 3; + private static final int DOCUMENT_3_ORDINAL = 5; + + @ClassRule + public static SearchSetupHelper setupHelper = new SearchSetupHelper(); + + private static final SimpleMappedIndex mainIndex = + SimpleMappedIndex.of( IndexBinding::new ).name( "main" ); + + @BeforeClass + public static void setup() { + setupHelper.start() + .withIndexes( mainIndex ) + .setup(); + + initData(); + } + + private final FieldTypeDescriptor fieldTypeDescriptor; + + public FieldSortDynamicFieldIT(FieldTypeDescriptor fieldTypeDescriptor) { + this.fieldTypeDescriptor = fieldTypeDescriptor; + } + + @Test + public void simple() { + String fieldPath = mainFieldPath(); + + assertThatQuery( matchNonEmptyQuery( f -> f.field( fieldPath ).asc() ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_1, DOCUMENT_2, DOCUMENT_3 ); + assertThatQuery( matchNonEmptyQuery( f -> f.field( fieldPath ).desc() ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_3, DOCUMENT_2, DOCUMENT_1 ); + } + + @Test + @TestForIssue(jiraKey = "HSEARCH-4531") + public void neverPopulated() { + assumeTrue( + "This backend doesn't support sorts on a field of type '" + fieldTypeDescriptor + + "' that is missing from some of the target indexes.", + TckConfiguration.get().getBackendFeatures() + .supportsFieldSortWhenFieldMissingInSomeTargetIndexes( fieldTypeDescriptor.getJavaType() ) + ); + + String neverPopulatedFieldPath = neverPopulatedFieldPath(); + String mainFieldPath = mainFieldPath(); + + // The field that wasn't populated shouldn't have any effect on the sort, + // but it shouldn't trigger an exception, either (see HSEARCH-4531). + assertThatQuery( matchNonEmptyQuery( f -> f.composite() + .add( f.field( neverPopulatedFieldPath ).asc() ) + .add( f.field( mainFieldPath ).asc() ) ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_1, DOCUMENT_2, DOCUMENT_3 ); + assertThatQuery( matchNonEmptyQuery( f -> f.composite() + .add( f.field( neverPopulatedFieldPath ).desc() ) + .add( f.field( mainFieldPath ).desc() ) ) ) + .hasDocRefHitsExactOrder( mainIndex.typeName(), DOCUMENT_3, DOCUMENT_2, DOCUMENT_1 ); + } + + private SearchQuery matchNonEmptyQuery( + Function sortContributor) { + return matchNonEmptyQuery( sortContributor, mainIndex.createScope() ); + } + + private SearchQuery matchNonEmptyQuery( + Function sortContributor, StubMappingScope scope) { + return scope.query() + .where( f -> f.matchAll().except( f.id().matching( EMPTY ) ) ) + .sort( sortContributor ) + .toQuery(); + } + + private String mainFieldPath() { + return mainFieldPath( fieldTypeDescriptor ); + } + + private static String mainFieldPath(FieldTypeDescriptor type) { + return IndexBinding.fieldPath( type, "main" ); + } + + private String neverPopulatedFieldPath() { + return IndexBinding.fieldPath( fieldTypeDescriptor, "neverPopulated" ); + } + + private static void initData() { + BulkIndexer mainIndexer = mainIndex.bulkIndexer() + // Important: do not index the documents in the expected order after sorts (1, 2, 3) + .add( DOCUMENT_2, document -> initDocument( document, DOCUMENT_2_ORDINAL ) ) + .add( EMPTY, document -> initDocument( document, null ) ) + .add( DOCUMENT_1, document -> initDocument( document, DOCUMENT_1_ORDINAL ) ) + .add( DOCUMENT_3, document -> initDocument( document, DOCUMENT_3_ORDINAL ) ); + mainIndexer.join(); + } + + private static void initDocument(DocumentElement document, Integer ordinal) { + for ( FieldTypeDescriptor type : supportedFieldTypes ) { + addValue( type, document, ordinal ); + } + } + + private static void addValue(FieldTypeDescriptor type, DocumentElement documentElement, Integer ordinal) { + if ( ordinal == null ) { + return; + } + documentElement.addValue( + mainFieldPath( type ), + type.getAscendingUniqueTermValues().getSingle().get( ordinal ) + ); + } + + private static class IndexBinding { + public static String fieldPath(FieldTypeDescriptor type, String suffix) { + return type.getUniqueName() + "_" + suffix; + } + + IndexBinding(IndexSchemaElement root) { + for ( FieldTypeDescriptor type : supportedFieldTypes ) { + root.fieldTemplate( "myTemplate" + type.getUniqueName(), + f -> type.configure( f ).sortable( Sortable.YES ) ) + .matchingPathGlob( fieldPath( type, "*" ) ); + } + } + } + +}