Let Elasticsearch sort by Byte and Short keys

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`,

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
      */

janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java

@@ -1684,16 +1684,28 @@ public Stream<String> 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";
         }
         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";
         }

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<String, KeyInformation> 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 {