Let Elasticsearch sort by Byte and Short keys

[porunov]

Fixes #5080

A graph query which a mixed index on Elasticsearch answered in the order of a Byte or a Short key failed with Could not call index, and an index query ordered by such a key failed too: ElasticSearchIndex 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), as convertToEsDataType had no type for these keys. This PR gives byte and short, the types which pushMapping maps them to. It also gives keyword in place of string for a string key, a type which Elasticsearch 5 replaced: Elasticsearch 8 and 9 rejected such a sort where the index had no mapping of the key.

Changes

• ElasticSearchIndex.convertToEsDataType, which gives the unmapped_type of a sort by a key, the type which Elasticsearch sorts by where an index has no mapping of the field:
  • gives short for a Short key and byte for a Byte key, the types which pushMapping maps them to. It returned null for them, and the request went out with "unmapped_type": null, which Elasticsearch rejects while it parses the request, whether the field is mapped or not;
  • gives keyword for every string key. An unmapped_type applies only to an index without a mapping of the field, whose documents all sort as having no value, so it needs only a type which Elasticsearch can sort strings by, whatever the key's mapping: pushMapping maps the field which such a sort reads as keyword, or as text where the key has a custom string analyzer. It gave string, which Elasticsearch 5 replaced with text and keyword: where the index had no mapping of the field, 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, [unmapped_type:string] should be replaced with [unmapped_type:keyword].
• Changelog: a section, "Elasticsearch sorts by Byte and Short keys".

Of the built-in types which ElasticSearchIndex supports, every one now has an unmapped type, with the PR which compares UUIDs as their string forms (branch order-uuids-as-the-indexes-do): it adds a branch for a UUID key, keyword, after the Geoshape one. The two merge cleanly in either order.

What the versions accept, sending the sorts by hand to a container of each, with an index which maps a keyword, a byte and a short field, and a field which it doesn't map:

Server, by the unmapped_type of the sort	null	byte, short, keyword	string, mapped field	string, unmapped field
Elasticsearch 6.6.0	400	accepted	accepted	accepted, with a deprecation warning
Elasticsearch 7.17.8	400	accepted	accepted	accepted, with a deprecation warning
Elasticsearch 8.15.3	400	accepted	accepted	400, No mapper found for type [string]
Elasticsearch 9.5.4	400	accepted	accepted	400, No mapper found for type [string]
OpenSearch 2.19.6	400	accepted	accepted	accepted
OpenSearch 3.9.0	400	accepted	accepted	accepted

The 400 for null is [field_sort] unmapped_type doesn't support values of type: VALUE_NULL, for a mapped field as for an unmapped one.

Tests

• JanusGraphIndexTest.testMixedIndexOrdersByBytesAndShorts (every index backend; the mixed index is named mixed, which the Solr test container has a collection for). A mixed index on tag, a Byte key small and a Short key medium; four vertices with the least value of each, 0, the greatest and -1. For each of the two keys, the graph query g.V().has('tag', 't').order().by(key) comes in the order of the values, ascending, descending and ascending with a limit of two, and so does the index query indexQuery('mixed', 'v.tag:t').orderBy(key, desc).
• ElasticsearchConfigTest.testOrderByStringKeyWhichTheIndexHasNotMapped. Under an external dynamic mapping (index.[X].elasticsearch.create.use-external-mappings) without index.[X].elasticsearch.create.allow-mapping-update, a string key registered with the index, and a document without it: the index has no mapping of the key, which the test checks, and a query ordered by the key returns the document.

Without the fix, on Elasticsearch 9.5.4:

• Without the byte and short cases: testMixedIndexOrdersByBytesAndShorts fails at its first query ordered by small, with Could not call index from the 400 for the unmapped_type of null.
• Without the short case alone: it fails at the first query ordered by medium, after those by small passed.
• With string for a string key: testOrderByStringKeyWhichTheIndexHasNotMapped fails with PermanentBackendException: Unknown exception while executing index operation, from the 400 No mapper found for type [string]. Elasticsearch 6 and 7 and OpenSearch accept string for a field without a mapping (see the table above), so there the test passes either way.
• Each of the two tests passes with the other's mutation.

Runs, locally, Java 11, on master dd4f0f845:

• BerkeleyElasticsearchTest (118, 1 skipped), ElasticsearchIndexTest (306) and ElasticsearchConfigTest (13) pass against Elasticsearch 9.5.4 in a container, with checkstyle.
• The two new tests pass against Elasticsearch 6.6.0, 7.17.8 and 8.15.3 and OpenSearch 2.19.6 and 3.9.0 in containers.
• janusgraph-driver (130), janusgraph-core (228), janusgraph-inmemory (652, 85 skipped), janusgraph-berkeleyje (350, 9 skipped) and janusgraph-lucene (394, 148 skipped) pass, with checkstyle.
• The Solr test container doesn't start on this machine (a 404 from /solr/admin/collections while it starts), so BerkeleySolrTest's configuration ran in the JVM against a MiniSolrCloudCluster (Solr 9.10.1) with the test schema: testMixedIndexOrdersByBytesAndShorts passes. The rest of the Solr suite is left to CI.

Notes

• Left out:
  • A custom attribute type which is a Number other than the six built-in ones: ElasticSearchIndex supports it, but pushMapping maps it to no type, so a sort by it still goes out with an unmapped_type of null. Any numeric type would do there, as it only applies where no document of the index holds the field; it is left to a change which maps such types.
  • Character keys, which ElasticSearchIndex doesn't support.
• OpenSearch accepts string, and keyword as well.

🤖 Generated with Claude Code

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings

• The PR description (and the method signature) indicates the returned string type should depend on… · New
• This test’s setup and comment state it runs 'Without allow-mapping-update', and later asserts the… · New
• idx is closed only on the happy-path; if an assertion fails (e.g., mapping unexpectedly includes… · New

What changed in this PR

Fix Elasticsearch/OpenSearch sorting failures when ordering by Byte, Short, or unmapped string keys by ensuring unmapped_type is set to a valid ES datatype.

Changes:

• Update ElasticSearchIndex.convertToEsDataType to return byte, short, and keyword instead of null/string for relevant key types.
• Add regression tests covering mixed-index ordering by Byte/Short and ordering by an unmapped string field under external dynamic mappings.
• Document the behavior change in the changelog.

File	Description
janusgraph-es/​src/​main/​java/​org/​janusgraph/​diskstorage/​es/​ElasticSearchIndex.java	Fixes ES unmapped_type derivation for Byte/Short and modernizes string type to keyword.
janusgraph-es/​src/​test/​java/​org/​janusgraph/​diskstorage/​es/​ElasticsearchConfigTest.java	Adds regression test ensuring ordering works when the index has no mapping for a string key.
janusgraph-backend-testutils/​src/​main/​java/​org/​janusgraph/​graphdb/​JanusGraphIndexTest.java	Adds mixed-index regression test for ordering by Byte and Short keys.
docs/​changelog.md	Documents the fix and relevant Elasticsearch version behavior.

🧠 Review effort: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings

• Using a fixed storeName increases the risk of cross-test interference (e.g., when test execution… · New
• If an exception is thrown by register(...) or add(...), the first IndexTransaction will… · New
• Nit: the new method header comment is hard to scan (very long lines) and is missing a space after… · New

3 resolved since last review

• idx is closed only on the happy-path; if an assertion fails (e.g., mapping unexpectedly includes…
• This test’s setup and comment state it runs 'Without allow-mapping-update', and later asserts the…
• The PR description (and the method signature) indicates the returned string type should depend on…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings

• The new comment implies the return value matches what pushMapping maps a sortable string field… · New
• Using a fixed storeName can make this test brittle if the suite reuses the same Elasticsearch… · New
• Minor style issue: add a space after // to match typical Java comment formatting (e.g., `//… · New

3 resolved since last review

• If an exception is thrown by register(...) or add(...), the first IndexTransaction will…
• Using a fixed storeName increases the risk of cross-test interference (e.g., when test execution…
• Nit: the new method header comment is hard to scan (very long lines) and is missing a space after…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open findings

• These // comments appear to violate common Java style and are very likely to violate Checkstyle… · New
• This introduces a magic timeout value in a new test. To keep tests easier to tune and less brittle,… · New

3 resolved since last review

• Using a fixed storeName can make this test brittle if the suite reuses the same Elasticsearch…
• The new comment implies the return value matches what pushMapping maps a sortable string field…
• Minor style issue: add a space after // to match typical Java comment formatting (e.g., `//…

🧠 Review effort: Lite