You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-ups from #6307: two of #6297's items lost their tracker, IN walks a range that answers in O(1), and EXPLAIN cannot describe the plan it would run #6323
Three follow-ups from #6307 (the #6297 / #6280 / #6279 / #6270 batch), filed together because that is where all three were found. Item 1 is a tracking regression created by that merge and is the reason this issue exists at all; items 2 and 3 are independent defects in the openCypher engine that the work happened to walk into.
1. Two of #6297's own items lost their tracker when #6297 was closed
#6297 carried four items. #6307 implemented items 1 and 2 and deliberately left items 3 and 4 alone, saying so in the PR description - but the commit's Closes #6297 trailer closed the issue anyway, and with it the only searchable record of the two that were not done:
Both now survive only as a comment in BaseRaftHATest (RESYNC_RETRY_TIMEOUT_MS at line 73, SLOW_WAIT_REPORT_MS at line 80) - which is exactly the condition #6297 was filed to escape, in its own words: "recorded in BaseRaftHATest's comment; this is just the tracking entry so it does not depend on someone rereading that file."
A search of open issues for RESYNC_RETRY_TIMEOUT_MS, reuseForks and the fork-isolation wording returns nothing, so this issue is now their tracker.
Worth folding in: two CI lanes are sized at the 60-minute job cap
Observed across the #6307 runs, and relevant to the fork-isolation item above because it is the same budget under pressure:
slow-unit-tests was cancelled at 1h0m16s in one run while progressing normally - no test failure, LSMTreeIndexTest had just completed - and passed in 46m on the most recent main run, having taken 57m40s and 39m27s on two earlier ones.
ha-integration-tests failed at exactly 1h0m8s, with 238 tests run and 1 error.
A lane whose honest duration sits within a few minutes of its own cap goes red on runner speed alone, and the failure reads as "the job failed" rather than "the job ran out of time", which is what made both look like regressions from the PR under test until the logs were pulled.
2. IN walks a range that can answer membership in O(1)
Found while diagnosing a CI failure in #6307: hugeRangeIsLazyWhenTheLimitIsDisabled measured 20,570 ms with 0 ms of JVM stall - not a pause, twenty seconds of real work - for three queries over range(0, 999999999). Timed individually:
8 ms RETURN size(range(0, 999999999)) AS r
1 ms RETURN range(0, 999999999)[123456789] AS r
3066 ms RETURN 999999998 IN range(0, 999999999) AS r
0 ms RETURN 5 IN range(0, 999999999) AS r
size() and indexing are answered from start, step and size alone, as GHSA-xmjm's lazy LongRangeList intends. IN is not: InExpression.evaluateTernary (engine/src/main/java/com/arcadedb/query/opencypher/ast/InExpression.java, the loop at ~line 105) walks the list element by element, so the cost is the position of the match - index 5 returns instantly, index 999,999,998 costs three seconds locally and about twenty on a CI runner.
The walk is not gratuitous, which is why this needs care rather than a one-line change. It exists for Cypher's three-valued logic: a match returns early, but a non-match has to know whether any element compared null-uncertain (foundNull) to decide between null and false, so a miss walks the whole list unconditionally. And the per-element comparison is valuesCompare, which delegates to the = operator's comparator for the temporal, array and map coercion and the NaN contract that #5293 established - semantics List.contains does not have.
So the opportunity is narrower than "call contains()": it is a typed fast path, not a general one.
LongRangeList.indexOf is documented and implemented as O(1) (an exact-multiple-of-step test, engine/src/main/java/com/arcadedb/utility/LongRangeList.java:106-119), and a range of longs can never contain null - so foundNull is statically false and both the match and the miss are answerable without walking.
But indexOf deliberately rejects Double/Float (return -1 at line 116-117), while Cypher's = coerces numerically. 5.0 IN range(0, 10) would therefore flip from true to false if the fast path were taken unconditionally. Any fix has to gate on the left operand's type, not only on the right operand's.
Worth fixing on its own terms - a user writing x IN range(...) against a large range has no way to know the cost depends on where the value sits - and it is currently costing the unit lane about 26 s in CypherRangeHeapExhaustionTest alone.
3. EXPLAIN cannot describe the plan it would run, for any query on the traditional path
CypherExecutionPlan.explain() (engine/src/main/java/com/arcadedb/query/opencypher/executor/CypherExecutionPlan.java:702) describes two things: the count push-down chain, and the cost-based physical plan. A query the optimizer does not claim gets neither, and falls to the branch at line 729:
EXPLAIN MATCH (a),(b) WHERE ID(a) = $sourceId AND ID(b) = $targetId RETURN a, b
OpenCypher Native Execution Plan
=================================
Using Traditional Execution (Non-Optimized)
Reason: Query pattern not yet supported by optimizer
Execution will use step-by-step interpretation
That is the whole answer - no steps, no order, nothing about what the query will actually do. profile() does not have this gap: its else branch at line 850 appends the real step chain, and PROFILE of the same query returns
Execution Plan (Traditional):
+ MATCH NODE (a) [id: #1:0]
+ MATCH NODE (b) [id: #4:0]
+ FILTER WHERE (ID(a) = $sourceId AND ID(b) = $targetId)
+ PROJECT RETURN a, b
+ FINAL PROJECTION [a, b]
so the information exists and is already formatted - appendStepChain is shared - it is simply not reached from explain().
The consequence is that EXPLAIN, the one command whose entire purpose is inspecting a plan without running it, is strictly less informative than PROFILE for exactly the queries a user is most likely to be investigating. The workaround is to run PROFILE, which executes: not an option for a slow query, and actively wrong for a writing one, since PROFILE bypasses the idempotency gate and a PROFILE MATCH ... SET ... does write.
It also undercuts the [id: ...] marker #6307 just added to MatchNodeStep.prettyPrint. That marker exists so a user can confirm the RID push-down fired, which is what #3216's resolution asks them to arrange - and on the traditional path, which is where those queries land, EXPLAIN will not show it. #6307's own regression test had to use PROFILE for this reason.
Suspected fix: build the step chain in explain() the way profile() already does and append it, rather than printing a reason and stopping. The steps are constructed without being pulled, so nothing executes.
Three follow-ups from #6307 (the #6297 / #6280 / #6279 / #6270 batch), filed together because that is where all three were found. Item 1 is a tracking regression created by that merge and is the reason this issue exists at all; items 2 and 3 are independent defects in the openCypher engine that the work happened to walk into.
1. Two of #6297's own items lost their tracker when #6297 was closed
#6297 carried four items. #6307 implemented items 1 and 2 and deliberately left items 3 and 4 alone, saying so in the PR description - but the commit's
Closes #6297trailer closed the issue anyway, and with it the only searchable record of the two that were not done:RESYNC_RETRY_TIMEOUT_MS. fix(#6267): a peer address that identifies nobody says so, and a membership test stops poisoning its own teardown #6284 took it from 120 s to 30 s on the evidence of nine fullha-integration-testsruns in which no wait exceeded the 10 s report threshold, and loweredSLOW_WAIT_REPORT_MSto 5 s to keep resolution for the next decision. Whether 30 s can come down further needs a handful of runs at the new threshold; there was no number to choose without them, which is why fix(#6297, #6280, #6279, #6270): a teardown that deletes what the test wrote, a warning that speaks again, and bounds that say what they are for #6307 did not choose one.RaftMigrationCompactionRaceITin the same reused JVM (failsafereuseForks=true/forkCount=1for the whole module). The Ratis-initiated snapshot install has neither guard the manual one has, so a follower can resync from its own address (or from the wrong peer) and report success #6202 removed the suspected cause and fix(#6267): a peer address that identifies nobody says so, and a membership test stops poisoning its own teardown #6284 cut the budget, but nothing has been done about the adjacency itself.Both now survive only as a comment in
BaseRaftHATest(RESYNC_RETRY_TIMEOUT_MSat line 73,SLOW_WAIT_REPORT_MSat line 80) - which is exactly the condition #6297 was filed to escape, in its own words: "recorded inBaseRaftHATest's comment; this is just the tracking entry so it does not depend on someone rereading that file."A search of open issues for
RESYNC_RETRY_TIMEOUT_MS,reuseForksand the fork-isolation wording returns nothing, so this issue is now their tracker.Worth folding in: two CI lanes are sized at the 60-minute job cap
Observed across the #6307 runs, and relevant to the fork-isolation item above because it is the same budget under pressure:
slow-unit-testswas cancelled at 1h0m16s in one run while progressing normally - no test failure,LSMTreeIndexTesthad just completed - and passed in 46m on the most recentmainrun, having taken 57m40s and 39m27s on two earlier ones.ha-integration-testsfailed at exactly 1h0m8s, with 238 tests run and 1 error.A lane whose honest duration sits within a few minutes of its own cap goes red on runner speed alone, and the failure reads as "the job failed" rather than "the job ran out of time", which is what made both look like regressions from the PR under test until the logs were pulled.
2.
INwalks a range that can answer membership in O(1)Found while diagnosing a CI failure in #6307:
hugeRangeIsLazyWhenTheLimitIsDisabledmeasured 20,570 ms with 0 ms of JVM stall - not a pause, twenty seconds of real work - for three queries overrange(0, 999999999). Timed individually:size()and indexing are answered from start, step and size alone, as GHSA-xmjm's lazyLongRangeListintends.INis not:InExpression.evaluateTernary(engine/src/main/java/com/arcadedb/query/opencypher/ast/InExpression.java, the loop at ~line 105) walks the list element by element, so the cost is the position of the match - index 5 returns instantly, index 999,999,998 costs three seconds locally and about twenty on a CI runner.The walk is not gratuitous, which is why this needs care rather than a one-line change. It exists for Cypher's three-valued logic: a match returns early, but a non-match has to know whether any element compared null-uncertain (
foundNull) to decide betweennullandfalse, so a miss walks the whole list unconditionally. And the per-element comparison isvaluesCompare, which delegates to the=operator's comparator for the temporal, array and map coercion and the NaN contract that #5293 established - semanticsList.containsdoes not have.So the opportunity is narrower than "call
contains()": it is a typed fast path, not a general one.LongRangeList.indexOfis documented and implemented as O(1) (an exact-multiple-of-step test,engine/src/main/java/com/arcadedb/utility/LongRangeList.java:106-119), and a range of longs can never contain null - sofoundNullis statically false and both the match and the miss are answerable without walking.indexOfdeliberately rejectsDouble/Float(return -1at line 116-117), while Cypher's=coerces numerically.5.0 IN range(0, 10)would therefore flip from true to false if the fast path were taken unconditionally. Any fix has to gate on the left operand's type, not only on the right operand's.Worth fixing on its own terms - a user writing
x IN range(...)against a large range has no way to know the cost depends on where the value sits - and it is currently costing the unit lane about 26 s inCypherRangeHeapExhaustionTestalone.3.
EXPLAINcannot describe the plan it would run, for any query on the traditional pathCypherExecutionPlan.explain()(engine/src/main/java/com/arcadedb/query/opencypher/executor/CypherExecutionPlan.java:702) describes two things: the count push-down chain, and the cost-based physical plan. A query the optimizer does not claim gets neither, and falls to the branch at line 729:That is the whole answer - no steps, no order, nothing about what the query will actually do.
profile()does not have this gap: itselsebranch at line 850 appends the real step chain, andPROFILEof the same query returnsso the information exists and is already formatted -
appendStepChainis shared - it is simply not reached fromexplain().The consequence is that
EXPLAIN, the one command whose entire purpose is inspecting a plan without running it, is strictly less informative thanPROFILEfor exactly the queries a user is most likely to be investigating. The workaround is to runPROFILE, which executes: not an option for a slow query, and actively wrong for a writing one, sincePROFILEbypasses the idempotency gate and aPROFILE MATCH ... SET ...does write.It also undercuts the
[id: ...]marker #6307 just added toMatchNodeStep.prettyPrint. That marker exists so a user can confirm the RID push-down fired, which is what #3216's resolution asks them to arrange - and on the traditional path, which is where those queries land,EXPLAINwill not show it. #6307's own regression test had to usePROFILEfor this reason.Suspected fix: build the step chain in
explain()the wayprofile()already does and append it, rather than printing a reason and stopping. The steps are constructed without being pulled, so nothing executes.