Repository navigation
Conversation
Fixes JanusGraph#5101 A graph query ordered by a key of several values, LIST or SET, failed with an IllegalStateException, "Multiple properties exist for the provided key", wherever JanusGraph compared an element with several values of the key itself: OrderList read the key with valueOrNull, which takes a single value. That was so where it sorted the results in memory, as those of a composite index; where it merged the vertices which a transaction added or changed with those of a mixed index, as a vertex which the transaction made match the query; and where it merged the queries of the branches of an or(). OrderList now compares such an element by the least of its values for an ascending order and by the greatest for a descending one, compared as any two values of the key are, strings by code point. Elasticsearch, OpenSearch and Solr sort such a key so by default, so the elements which JanusGraph places come where the index puts the others. An element without a value of the key still comes last. Lucene keeps no doc values for a key of several values, so a query which a Lucene index answered, ordered by such a key, failed with "Could not call index", as did a traversal's order of edges, which it folds in whatever the cardinality of the key. An index whose backend can't sort by such a key no longer takes the order, and JanusGraph sorts the results itself; IndexSelectionUtil.indexCoversOrder takes the IndexSerializer for that, and its old form is deprecated. GraphCentricQueryBuilder holds the indexes which a strategy selects to sort the results only where the first of them was given the query's order, each of its keys in its direction, so that results which a custom strategy holds sorted by the old form, or by an index given only part of the order, are sorted too. A vertex whose values of such a key a transaction changed kept the place which an index that sorts by the key gave it, as JanusGraph couldn't compare it; it now comes where the values it holds place it, as for a key of one value. Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-backend ordering changes and the still-open #5100 prerequisite require human integration and merge-sequencing verification.
0 open findings
What changed in this PR
Fixes #5101 by aligning JanusGraph’s in-memory ordering of multi-valued properties with mixed-index ordering, extending transaction-ordering corrections from #5023.
Changes:
- Uses the minimum value ascending and maximum descending for
LISTandSETkeys. - Falls back to in-memory sorting when an index cannot provide the complete requested order.
- Adds regression tests and documents ordering semantics and compatibility implications.
| File | Description |
|---|---|
| janusgraph-test/src/test/java/org/janusgraph/graphdb/query/QueryTest.java | Tests multi-value comparisons and Unicode ordering. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/StandardJanusGraphTx.java | Repositions vertices with changed multi-valued order keys. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/QueryProcessor.java | Reads limit-bound values through the order definition. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/index/IndexSelectionUtil.java | Checks backend support for multi-valued ordering. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/index/BruteForceIndexSelectionStrategy.java | Uses capability-aware order coverage. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/index/ApproximateIndexSelectionStrategy.java | Accounts for ordering capabilities during selection. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/graph/GraphCentricQueryBuilder.java | Verifies the index receives the complete requested order. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/internal/OrderList.java | Selects minimum or maximum property values for comparison. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/database/IndexSerializer.java | Omits unsupported backend orders. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/PartialOrderIndexSelectionStrategy.java | Supplies partial ordering for regression tests. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java | Tests backend ordering, transaction changes, and fallbacks. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/DeprecatedOrderCoverageIndexSelectionStrategy.java | Exercises legacy order-coverage behavior. |
| docs/schema/index-management/index-performance.md | Explains multi-valued ordering and traversal limitations. |
| docs/changelog.md | Documents behavior changes and custom-backend compatibility. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5101
A graph query ordered by a key of several values,
LISTorSET, failed withIllegalStateException: Multiple properties exist for the provided keywherever JanusGraph compared an element with several values of the key itself, asOrderListread the key withvalueOrNull, which takes a single value:g.V().order().by('names'), which no index answers, on a graph whose mixed index backends all sort by such a key;or(), such asg.V().or(has('uid', 'u1'), has('uid', 'u2')).order().by('names')on such a graph.On Lucene, which keeps doc values only for keys of one value, a query of the API ordered by such a key, which a Lucene index holding the key answered, failed with
JanusGraphException: Could not call index, caused byIllegalStateException: unexpected docvalues type NONE for field 'names' (expected=SORTED); so did a traversal's order of edges by a key declaredLIST, which a traversal folds in whatever the cardinality of the key, as an edge holds one value of a key at most.JanusGraph now compares such an element by the least of its values in an ascending order and by the greatest in a descending one, which is how Elasticsearch and OpenSearch sort a field of several values by default (JanusGraph sets no sort
mode), and Solr a multi-valued field, so that the elements which JanusGraph places come where the index puts the others. An element without a value of the key still comes last. An index whose backend can't sort by such a key, as Lucene's, no longer takes the order, and JanusGraph sorts the results itself.Changes
OrderList.OrderEntry.value(element)(new) is the value by which the order compares an element: the element's value of the key, or, for a key ofLISTorSETcardinality, the first of its values in the order, compared as any two values of the key are (compareOrderValues: strings by code point), so the least for an ascending order and the greatest for a descending one;nullwhere it has none.OrderEntry.compare,OrderList.values()and the newOrderList.value(element, position)read the values through it. Whether a key holds several values is decided once, when the order is built.QueryProcessorreads the value of the first order key of the last new element within the limit through the order as well, which it read withvalueOrNulland failed on. The range which Find the vertices a transaction changed for its graph queries by key and value #5023 then gives the index query still applies to keys of one value only.IndexSelectionUtil.indexCoversOrder(MixedIndexType, OrderList, IndexSerializer)(new) holds an index to sort by the order only where its backend can sort by each of its keys of several values (IndexFeatures.supportsOrderingListProperty), which Lucene's can't.IndexSerializer.getQueryand the brute force and approximate index selection strategies use it; the old form is deprecated.GraphCentricQueryBuilderholds the indexes which a strategy selects to sort the results only where the first of them was given the query's order, the same keys in the same directions (sortedOnlyWhereTheIndexSortsandtakesTheOrder, new). A custom strategy which decides by the deprecated two-argument form holds an index which wasn't given the order sorted, and one could give the index only part of the order; their results, in the index's own order or with the ties of that part in it, would have gone through as sorted. JanusGraph sorts them now.StandardJanusGraphTxno longer leaves the vertices whose values of a key of several values the transaction changed where an index which sorts by the key places them, by their values before the transaction (hasOrderKeysOfOneValue, removed): as for a key of one value, the query takes them among its new vertices and places them by the values they hold now, whichOrderListcan now compare.Tests
QueryTest.testOrderListComparesAKeyOfSeveralValuesByItsLeastOrGreatestValue(inmemory, the comparison itself): anOrderListby aLISTkey of strings and by aSETkey of integers, ascending and descending, compares a vertex of several values by its least or greatest one, picked by code point (U+FFFF before U+1F600, whichString.compareToputs first), places a vertex without a value last, gives the same order throughvalues()andcompareValues(), counts a value which the vertex gains, and orders the ties of a key of several values by a key of one value.JanusGraphIndexTest.testOrderByAKeyOfSeveralValuesComparesItsLeastOrGreatestValue(every index backend; the mixed index is namedmixed, which the Solr test container creates). Vertices with aLISTkeynamesand aSETkeyscoresof several values, one with one value, and one with none:graph.query()orders bynamesand byscores, of what the mixed index returns and of what a composite index returns, ascending and descending, by the least and the greatest value, the vertex without a value last; so do traversals and anor()of branches which a composite index answers where every mixed index backend sorts by such a key. Then a transaction makes a vertex with two names match the query, and gives another one a further name: the queries, a limited one included, and the traversals place them by the values they hold now.JanusGraphIndexTest.testOrderingListProperty: where every mixed index backend sorts by a key of several values,g.V().order().by('name2'), which no index answers, is sorted by the least value ascending and by the greatest descending, as the index sorts it, rather than failing; elsewhere TinkerPop still fails on it.JanusGraphIndexTest.testResultsWhichAStrategyHoldsSortedByAnIndexWithoutTheOrderAreSortedInMemory(every index backend): withquery.index-select-strategyset to a strategy which decides by the deprecatedindexCoversOrder(MixedIndexType, OrderList)(DeprecatedOrderCoverageIndexSelectionStrategy, new, in the test utilities), a query ordered by aLISTkey, ascending and descending, which the strategy holds the mixed index to sort, comes sorted on Lucene too.JanusGraphIndexTest.testResultsWhichAStrategyHoldsSortedByAnIndexGivenPartOfTheOrderAreSortedInMemory(every index backend): with a strategy which gives the mixed index only the first key of the order (PartialOrderIndexSelectionStrategy, new, in the test utilities), a query ordered bynameand then byage, ascending and descending, comes sorted by both, where the index sorts the ties ofnamein an order of its own, which can't be both that of their ages and its reverse.JanusGraphIndexTest.testOrderOfEdgesByAKeyDeclaredToHoldSeveralValues(every index backend): edges of a key declaredLISTin the mixed indexedgesare ordered by it, ascending and descending, by a traversal, and bygraph.query(), which places the edge without a value last in both directions.JanusGraphIndexTest.testTransactionOrdersVerticesByTheValuesOfAKeyOfSeveralValuesWhichTheyHoldNow, Find the vertices a transaction changed for its graph queries by key and value #5023'stestTransactionKeepsTheIndexOrderOfAKeyOfSeveralValuesrenamed: the values which the transaction gives the vertices turn the index's order around, ascending and descending, also next to a key of one value which it changed, where the test checked only which vertices came.On master
dd4f0f845, with these tests:testOrderByAKeyOfSeveralValuesComparesItsLeastOrGreatestValuefails with theIllegalStateExceptionat the query of the composite index,testOrderingListPropertywith it at the order which no index answers, and the renamed test of Find the vertices a transaction changed for its graph queries by key and value #5023 gets the index's order instead of the transaction's. A probe on master fails the same way for theor()of branches, and for each query in the transaction which made a vertex of two names match; the orders which Elasticsearch sorts agree with the least and the greatest value.testOrderByAKeyOfSeveralValuesComparesItsLeastOrGreatestValuefails withCould not call index, caused byunexpected docvalues type NONE for field 'names' (expected=SORTED).QueryTest's test fails with theIllegalStateException.Could not call index.Mutations, each reverting one part of the change, are all caught:
valueOrNull, as before: the unit test and the index test on Lucene fail with theIllegalStateException.Could not call index.[a, d, b, c]on Lucene, its index's own order.nameand then byagedescending returns[1, 2, 3, 0]on Lucene, the ties ofnamein the index's order.valueOrNull, as before: on Elasticsearch, the index test's limited query in the transaction fails with theIllegalStateException.Locally, Java 11, with checkstyle (no violations), before the review's changes below:
-Dtest.skip.tp=false,InMemoryJanusGraphProcessTestandInMemoryMultiQueryJanusGraphProcessTest, 1,860 tests, 80 skipped) pass.BerkeleyElasticsearchTest(120, 1 skipped) passes against Elasticsearch 9.5.4 in a container.MiniSolrCloudCluster, as the Solr test container doesn't start here). Solr sorts a multi-valued field by its least value ascending and its greatest descending, and the schemas JanusGraph ships and tests with put the documents without a value last (sortMissingLast). Solr's CI runs them in the container.QueryTestandJanusGraphStepStrategyTest(91) and the six index tests of keys of several values pass on Lucene, and the six on Elasticsearch 9.5.4 in a container; on Solr in the JVM, the two strategy tests and the order of edges pass.Notes
order().by()of vertices by a key of several values only where every mixed index backend can sort by it, as before; elsewhere TinkerPop sorts, and fails on a vertex with several values of the key. An order of edges they fold whatever the cardinality of the key, as before, as an edge holds one value of a key at most.LISTvalue the transaction extended,g.V().has('kind', 'k').order().by('several', desc).limit(10)took 234 ms the first time and then 42 and 37 ms, with the right vertices; master took 30, 8 and 6 ms and returned the wrong ones (z04656, z04081, z04196, where the values which the transaction gave putz04999, z04998, z04997first), and the same query by a key of one value, which takes Find the vertices a transaction changed for its graph queries by key and value #5023's bound, 59, 39 and 34 ms.withoutResultsAfter). With the least or greatest value it could apply to keys of several values too, as a range on such a field holds where any value is in it: the least value at most the bound ascending, the greatest at least it descending. Left as a follow-up.graph.query()now gives an order by a key of several values only to an index whose backend declaresIndexFeatures.supportsOrderingListProperty, where master gave it to any index holding the key. A backend which can sort by such a key but doesn't declare it now gets every match read and sorted by JanusGraph, which fails with aQueryExceptionat 1,000,000 matches; the changelog says so.QueryProcessor'sPreSortingIterator, which reads the order key of each vertex the index returns, a vertex which the transaction removed included: on this PR alone, aftertx.getVertex(c).remove(),tx.query().has('gender', 'female').orderBy('names', asc).vertices()fails withVertex with id N was removed, where master failed withCould not call index, so it's no regression for the backends which JanusGraph ships. With Leave out removed vertices before sorting a query in memory #5100, which leaves out the removed vertices before it sorts, it returns[a, b, d]. The changelog's "JanusGraph now sorts the results of such a query itself" holds in a transaction which removed such a vertex once Leave out removed vertices before sorting a query in memory #5100 is in.compareOrderValues, the value of several which an element is ordered by is picked in that order too; the two merge without conflicts.🤖 Generated with Claude Code