diff --git a/docs/changelog.md b/docs/changelog.md index b0490cc606..f4c1242892 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -895,6 +895,21 @@ where the same failure during a commit is reattempted. It now runs through `Back reattempted with backoff for up to `storage.write-time`, and only then reported. A restore writes whole documents, so a reattempt is idempotent. +##### Elasticsearch sorts by Byte and Short keys + +A graph query which a mixed index on Elasticsearch answered in the order of a `Byte` or a `Short` key, such as +`g.V().has('tag', 't').order().by('small')`, failed with `Could not call index`, and an index query (`indexQuery`) +with `orderBy` on such a key failed too: JanusGraph gave the sort an `unmapped_type` of `null`, which Elasticsearch and +OpenSearch reject with a 400 (`[field_sort] unmapped_type doesn't support values of type: VALUE_NULL`). The sort now +names the `byte` or `short` type which JanusGraph maps such a key to. + +A sort by a string key named the type `string`, which Elasticsearch 5 replaced with `text` and `keyword`. Where the +index had no mapping of the key, as under an external dynamic mapping +(`index.[X].elasticsearch.create.use-external-mappings` without `index.[X].elasticsearch.create.allow-mapping-update`) +before any document held the key, Elasticsearch 8 and 9 rejected the sort (`No mapper found for type [string]`), and +Elasticsearch 6 and 7 sorted by `keyword` in its place, with a deprecation warning. It now names `keyword`, the type +which JanusGraph maps the field a sort by a string key reads to, unless the key has a custom string analyzer. + ##### Transient Elasticsearch read failures are reattempted A mixed index query, count or aggregation against Elasticsearch wrapped every failure in a `PermanentBackendException`, diff --git a/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java b/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java index 8ae53b864d..bda87aff85 100644 --- a/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java +++ b/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java @@ -873,6 +873,39 @@ private void checkIndexingCounts(String[] words, int numV, int originalNumV, boo assertCount(numV, tx.query().has("name", Cmp.LESS_THAN_EQUAL, "world").edges()); } + /** + * A mixed index sorts by a Byte or a Short key, as by any other whole number, for a graph query and for a direct + * index query. Elasticsearch failed such a sort, which JanusGraph gave an unmapped type of null. + */ + @Test + public void testMixedIndexOrdersByBytesAndShorts() { + final PropertyKey tag = makeKey("tag", String.class); + final PropertyKey small = makeKey("small", Byte.class); + final PropertyKey medium = makeKey("medium", Short.class); + mgmt.buildIndex("mixed", Vertex.class).addKey(tag, getStringMapping()).addKey(small).addKey(medium) + .buildMixedIndex(INDEX); + finishSchema(); + + final Object min = tx.addVertex("tag", "t", "small", Byte.MIN_VALUE, "medium", Short.MIN_VALUE).id(); + final Object zero = tx.addVertex("tag", "t", "small", (byte) 0, "medium", (short) 0).id(); + final Object max = tx.addVertex("tag", "t", "small", Byte.MAX_VALUE, "medium", Short.MAX_VALUE).id(); + final Object minusOne = tx.addVertex("tag", "t", "small", (byte) -1, "medium", (short) -1).id(); + tx.commit(); + + newTx(); + for (final String key : Arrays.asList("small", "medium")) { + assertEquals(Arrays.asList(min, minusOne, zero, max), + tx.traversal().V().has("tag", "t").order().by(key, asc).id().toList(), key); + assertEquals(Arrays.asList(max, zero, minusOne, min), + tx.traversal().V().has("tag", "t").order().by(key, desc).id().toList(), key); + assertEquals(Arrays.asList(min, minusOne), + tx.traversal().V().has("tag", "t").order().by(key, asc).limit(2).id().toList(), key); + assertEquals(Arrays.asList(max, zero, minusOne, min), + tx.indexQuery("mixed", "v.tag:t").orderBy(key, desc).vertexStream() + .map(result -> result.getElement().id()).collect(Collectors.toList()), key); + } + } + /** * Tests indexing boolean */ diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java index efa7237a5f..f8acc60644 100644 --- a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java +++ b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java @@ -1684,9 +1684,15 @@ public Stream query(IndexQuery query, KeyInformation.IndexRetriever info } } + //The unmapped_type of a sort by a key of the data type: the type which Elasticsearch takes the field for in an + //index without a mapping of it, where no document holds a value of it. It is the type which pushMapping maps a + //sortable field of the data type to, but keyword for any string key, which pushMapping maps to text where it has a + //string analyzer: with no values to sort, any type which Elasticsearch can sort by will do. Elasticsearch rejects a + //sort whose unmapped_type is null, and from version 8 on, where the index has no mapping of the field, one whose + //unmapped_type is "string", which versions 6 and 7 read as keyword with a deprecation warning private String convertToEsDataType(Class dataType, Mapping mapping) { if(String.class.isAssignableFrom(dataType)) { - return "string"; + return "keyword"; } else if (Integer.class.isAssignableFrom(dataType)) { return "integer"; @@ -1694,6 +1700,12 @@ else if (Integer.class.isAssignableFrom(dataType)) { else if (Long.class.isAssignableFrom(dataType)) { return "long"; } + else if (Short.class.isAssignableFrom(dataType)) { + return "short"; + } + else if (Byte.class.isAssignableFrom(dataType)) { + return "byte"; + } else if (Float.class.isAssignableFrom(dataType)) { return "float"; } diff --git a/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/ElasticsearchConfigTest.java b/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/ElasticsearchConfigTest.java index 06b21db23f..fa97f3de5b 100644 --- a/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/ElasticsearchConfigTest.java +++ b/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/ElasticsearchConfigTest.java @@ -14,6 +14,7 @@ package org.janusgraph.diskstorage.es; +import com.google.common.collect.ImmutableList; import org.apache.commons.io.IOUtils; import org.apache.http.HttpHost; import org.apache.http.client.methods.CloseableHttpResponse; @@ -27,8 +28,10 @@ import org.apache.http.util.EntityUtils; import org.apache.tinkerpop.shaded.jackson.core.type.TypeReference; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; +import org.janusgraph.core.Cardinality; import org.janusgraph.core.JanusGraph; import org.janusgraph.core.JanusGraphFactory; +import org.janusgraph.core.attribute.Cmp; import org.janusgraph.core.attribute.Text; import org.janusgraph.core.schema.Mapping; import org.janusgraph.diskstorage.BackendException; @@ -47,9 +50,11 @@ import org.janusgraph.diskstorage.indexing.IndexQuery; import org.janusgraph.diskstorage.indexing.IndexTransaction; import org.janusgraph.diskstorage.indexing.KeyInformation; +import org.janusgraph.diskstorage.indexing.StandardKeyInformation; import org.janusgraph.diskstorage.util.StandardBaseTransactionConfig; import org.janusgraph.diskstorage.util.time.TimestampProviders; import org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration; +import org.janusgraph.graphdb.internal.Order; import org.janusgraph.graphdb.query.condition.PredicateCondition; import org.janusgraph.util.system.ConfigurationUtil; import org.junit.jupiter.api.AfterEach; @@ -72,6 +77,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; +import java.util.stream.Collectors; import java.util.stream.Stream; import static org.janusgraph.diskstorage.es.ElasticSearchIndex.ALLOW_MAPPING_UPDATE; @@ -80,6 +86,7 @@ import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.INDEX_HOSTS; import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.INDEX_PORT; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -259,6 +266,65 @@ public void testUpdateExternalDynamicMapping(Boolean useMappingsForES7) throws E testExternalDynamic(true, useMappingsForES7); } + /** + * A sort by a string key which the index has no mapping for yet, as under an external dynamic mapping before any + * document holds the key, finds the documents. JanusGraph gave such a sort the unmapped type "string", which + * Elasticsearch 5 replaced with text and keyword: Elasticsearch 8 and newer reject it, and Elasticsearch 6 and 7 + * read it as keyword with a deprecation warning. + */ + @Test + public void testOrderByStringKeyWhichTheIndexHasNotMapped() throws Exception { + final String storeName = "test_mapping"; + final Configuration indexConfig = esr + .setConfiguration(GraphDatabaseConfiguration.buildGraphConfiguration(), INDEX_NAME) + .set(USE_EXTERNAL_MAPPINGS, true, INDEX_NAME) + .set(ALLOW_MAPPING_UPDATE, false, INDEX_NAME) + .restrictTo(INDEX_NAME); + final IndexProvider idx = open(indexConfig); + try { + orderByStringKeyWhichTheIndexHasNotMapped(idx, indexConfig, storeName); + } finally { + idx.close(); + } + } + + private void orderByStringKeyWhichTheIndexHasNotMapped(IndexProvider idx, Configuration indexConfig, + String storeName) throws Exception { + if (isMappingUsed(idx)) { + executeRequestWithStringEntity(idx, "janusgraph_" + storeName, readTypesMapping("/dynamic_mapping.json")); + } else { + executeRequestWithStringEntity(idx, "janusgraph_" + storeName, + readTypelessMapping("/typeless_dynamic_mapping.json")); + } + final Map keys = new HashMap<>(IndexProviderTest.getMapping(idx.getFeatures(), + ANALYZER_ENGLISH, ANALYZER_KEYWORD, preferredGeoShapeMapping())); + keys.put("nickname", + new StandardKeyInformation(String.class, Cardinality.SINGLE, Mapping.STRING.asParameter())); + final KeyInformation.IndexRetriever indexRetriever = IndexProviderTest.getIndexRetriever(keys); + final BaseTransactionConfig txConfig = StandardBaseTransactionConfig.of(TimestampProviders.MILLI); + final Duration maxWrite = Duration.ofMillis(2000L); + IndexTransaction itx = new IndexTransaction(idx, indexRetriever, txConfig, maxWrite); + // Without allow-mapping-update, the dynamic mapping maps the key only once a document holds it + idx.register(storeName, "nickname", keys.get("nickname"), itx); + itx.add(storeName, "doc", IndexProviderTest.TIME, 1L, true); + itx.commit(); + try (ElasticSearchClient client = ElasticSearchSetup.REST_CLIENT.connect(indexConfig).getClient()) { + final IndexMapping mapping = client.getMapping("janusgraph_" + storeName, storeName); + assertFalse(mapping.getProperties().containsKey("nickname"), mapping.getProperties().toString()); + } + + itx = new IndexTransaction(idx, indexRetriever, txConfig, maxWrite); + final IndexQuery byNickname = new IndexQuery(storeName, + PredicateCondition.of(IndexProviderTest.TIME, Cmp.EQUAL, 1L), + ImmutableList.of(new IndexQuery.OrderEntry("nickname", Order.ASC, String.class))); + try { + assertEquals(Collections.singletonList("doc"), + itx.queryStream(byNickname).collect(Collectors.toList())); + } finally { + itx.rollback(); + } + } + @ParameterizedTest @MethodSource("useMappingsForES7Configuration") public void testExternalMappingsViaTemplate(Boolean useMappingsForES7) throws Exception {