Skip to content

fix(cypher): walk operator chains and UNION branches in loops - #2477

Open
DeusData wants to merge 2 commits into
mainfrom
fix/cypher-chain-depth
Open

DeusData wants to merge 2 commits into
mainfrom
fix/cypher-chain-depth

Conversation

@DeusData

@DeusData DeusData commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Long flat Cypher operator chains and UNION queries can exhaust the daemon worker stack even when their nesting is shallow. Parse and walk these chains in loops so stack use follows nested expressions rather than operand or branch count.

  • Link each AND/OR/XOR chain along its right child, and iterate it during evaluation and seed-selectivity planning. Preserve operand order, short-circuit behavior, XOR parity, and missing-value handling.
  • Parse UNION branches with one parser and an iterative branch loop; update the recursion whitelist accordingly.
  • Exercise 50,000-operand chains and 20,000 UNION ALL branches on a worker-sized stack, with exact row and ordering assertions. The large UNION fixture uses a thread-local test clock and its explicit 20,000-row output budget, so instrumentation overhead does not consume its deadline or reserve the default 100,000 binding slots per branch. Production deadlines continue to use the real clock.
  • Keep the deadline-abort and normal-query tests, add an exact positive-budget expiry test, and retain the 50-branch ordering, deduplication, and malformed-input controls.

Validation: the revised canonical Linux MSan Cypher suite passes all 215 tests without sanitizer diagnostics in 13 minutes 7 seconds, within the unchanged 30-minute runner bound. Native LLVM ASan/LSan fixed and restored runs also pass all 215 tests. A controlled expired clock reproduces only the deadline failure; restoring only the original UNION parser reproduces only the constrained-stack overflow (214 passed, one expected failure in each revert phase). Final lint passes. Further local CI was waived for this continuation; GitHub checks will validate the published revision.

The constrained-stack Unix tests use 256 KiB. The existing Windows test helper reserves 8 MiB, so those cases check behavior there but do not establish the same stack-overflow reproduction.

@DeusData

DeusData commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Checkpoint and handover (2026-10-03 UTC)

Published head: 400eb6d3f60f6de4cc26380831611481586f8266. The remote head was verified.

The deep-UNION regression uses a thread-local frozen deadline clock for the 20,000-UNION worker, while retaining the named deadline tests and row/stack assertions. Historical revised binding evidence included 215 native ASan/LSan and 215 Linux MSan tests passing. Preserve the distinction between deterministic structural coverage and the separate deadline tests.

Hosted snapshot at 2026-10-03 21:54:08 UTC: 1 in progress, 14 queued, 3 skipped, 20 success. Confirm the required checks on this exact head before treating it as ready.

No additional local build, test, lint, sanitizer, benchmark or CI runs were performed at this checkpoint, as requested. Earlier executed evidence remains historical; prepared tests and the newer source-reviewed changes must still be validated by the hosted gate.

The campaign is paused at the maintainer’s request. Local monitoring has stopped; hosted jobs remain running. No merge was performed. Thanks for reviewing this change.

A WHERE chain such as `a OR b OR c ...` was folded into a left-deep tree,
so every walker over the expression tree recursed once per operand: the
evaluator, the seed-selectivity pass of the planner and, every 128 nodes,
the free routine. UNION branches were parsed by a nested cbm_parse call
per branch, so the parser recursed once per branch as well. Stack use
therefore followed the length of a query, not its nesting, and the
daemon evaluates queries on a worker thread with a 256 KiB stack. A long
but flat query, such as a few thousand `OR name = ...` terms, needs to
work there.

Chains of one operator are now linked along the right child at parse
time, OP(a, OP(b, OP(c, ...))), and eval_expr and cypher_expr_selectivity
follow the links in a loop, recursing only into the operands, whose
nesting the existing parse-depth cap bounds. Operand order and the
left-to-right short-circuit of AND and OR are the same as in the nested
form, XOR keeps its running parity, and a missing value takes part in a
chain exactly as before. expr_free already walks with an explicit stack
and needs no change for a right-linked chain.

UNION branches are read by one parser in a loop in cbm_parse and linked
as they come; parse_post_where no longer parses the branch after the
keyword, and the ALL flag still sits on the branch before it.

Tests: 50,000-operand OR and AND chains and 20,000 UNION ALL branches run
on a thread with the worker stack size inside a forked child, with the
expected rows computed by the test; a chain with operands over a missing
property is compared with its parenthesised forms in both associations;
50 UNION and UNION ALL branches return the right rows.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Use a thread-local test clock for the large UNION structural regression
so sanitizer overhead cannot consume its execution deadline. Preserve
all row, ordering, and constrained-stack checks. Add an exact positive
budget expiry case alongside the existing deadline tests.

Request the fixture's exact 20000-row budget to avoid reserving the
default 100000 binding slots for every branch, and assert no truncation.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData force-pushed the fix/cypher-chain-depth branch from 400eb6d to c4d738b Compare October 4, 2026 18:28

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant