Repository navigation
fix(cypher): stop the early WHERE pruning rows it cannot judge yet - #2253
Merged
Merged
Conversation
Distilled from #1245 by Andrew Hundt (upstream b835b5e), re-derived on main's cypher.c rather than cherry-picked -- the file has moved ~9k lines since the branch point. The WHERE clause runs twice: once early, on seed rows where only the first alias is bound, to prune; and once late, when every alias is bound, to decide. The early pass used the same two-valued evaluator as the late one. That evaluator answers "true" for a comparison on an alias it cannot see -- the permissive default that makes the late pass work -- but NOT and XOR invert whatever they are given, so MATCH (a)-[:CALLS]->(b) WHERE NOT b.name CONTAINS "x" evaluated NOT(true) on every seed and pruned every row. Zero results, no error, for a query that is correct. The early pass now uses a three-valued evaluator. A condition on an alias that is not yet bound is UNKNOWN; NOT of UNKNOWN stays UNKNOWN; AND and OR short-circuit only on a definite operand; XOR with any UNKNOWN operand is UNKNOWN. Only a definite FALSE prunes. The late pass is unchanged and still gives the final two-valued answer, so nothing that was correct before changes. Three deliberate differences from the upstream diff: - the unbound-alias rule lives in one leaf, eval_condition_partial, so the expression-tree path and main's legacy flat AND/OR path cannot drift apart; - that leaf guards c->variable == NULL. On main a condition whose left side is a function of literals, coalesce(0, 1) >= 2, carries no variable at all; the upstream version would have passed NULL to strcmp; - the flat path keeps main's rule that an empty condition list is true. RED before the fix, all four with row_count == 0, expected 1: cypher_exec_where_not_on_relationship_target, cypher_exec_where_mixed_alias_and, ..._or, ..._xor. GREEN after: cypher 208 passed; cypher + mcp 526 passed, 4 skipped (the Windows-only skips). eval_expr_partial recurses and is added to CBM_RECURSION_WHITELIST. Co-authored-by: Andrew Hundt <ATHundt@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added
bug
DeusData
force-pushed
the
distill/1245-cypher-partial-where
branch
from
October 1, 2026 21:25
458b0db to
dc5293d
Compare
2 of 4 tasks
3 tasks done
DeusData
added a commit
to lorenzozanee/codebase-memory-mcp
that referenced
this pull request
Oct 20, 2026
…rseded The early WHERE pass in execute_single runs on seed rows where only the first alias is bound, so a NOT on the target node or the relationship pruned every row (DeusData#2250). This branch fixed that by skipping the early pass until every alias the WHERE reads was bound. While it waited for review, DeusData#2253 (8ab555e) landed the same fix on main as a three-valued early pass: an unbound alias is UNKNOWN, NOT keeps it UNKNOWN, and only a definite FALSE prunes. This branch's regression tests pass on main's production code unchanged, so restore src/cypher/cypher.c to the branch's base and keep the tests; the merge of main that follows takes main's implementation. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added a commit
to lorenzozanee/codebase-memory-mcp
that referenced
this pull request
Oct 20, 2026
Conflict in tests/test_cypher.c: both sides added WHERE NOT regression tests right after cypher_exec_where_not. Kept both sets, main's four DeusData#2253 tests first, then this branch's four, with their RUN_TEST lines in the same order. src/cypher/cypher.c merged cleanly to main's version. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added a commit
to lorenzozanee/codebase-memory-mcp
that referenced
this pull request
Oct 20, 2026
…eads eval_condition_partial (from DeusData#2253) answers UNKNOWN for a condition whose alias is not bound yet, but it only looked at c->variable. A function condition is filed under its first argument's alias, so in MATCH (a)-[:CALLS]->(b) WHERE coalesce(a.no_such_prop, b.name) = 'x' the seed row saw a bound a, read coalesce(NULL, <unbound b>) as a definite mismatch and pruned every row before CALLS expansion: zero rows, while the same predicate after WITH returned the right ones. Check every function argument's alias as well, the way this branch's original fix did (condition_bindings_available), inside main's three-valued leaf. RED before: cypher_exec_where_func_arg_on_unbound_target, row_count 0, expected 1. GREEN after; cypher suite 237 passed. Co-authored-by: lorenzozanee <wyz0707@proton.me> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.
Distilled from #1245 by @ahundt (upstream b835b5e), carried with
Co-authored-by. Re-derived on main'scypher.crather than cherry-picked — the file has moved ~9k lines since the branch point. #1245 stays open until its distills land.The bug
WHERE runs twice: early on seed rows (only the first alias bound) to prune, and late (all aliases bound) to decide. Both passes used the same two-valued evaluator, which answers true for a comparison on an alias it cannot see — the permissive default the late pass relies on. NOT and XOR invert whatever they are given, so:
evaluated
NOT(true)on every seed and pruned every row. Zero results, no error, for a correct query.The fix
The early pass now uses a three-valued evaluator: unbound alias → UNKNOWN; NOT of UNKNOWN stays UNKNOWN; AND/OR short-circuit only on a definite operand; XOR with any UNKNOWN is UNKNOWN. Only a definite FALSE prunes. The late pass is untouched, so anything that was correct before is unchanged.
Three deliberate differences from upstream: one shared leaf rule for the tree path and main's legacy flat AND/OR path; a
c->variable == NULLguard (on main a literal-only condition likecoalesce(0, 1) >= 2carries no variable — upstream would have passed NULL tostrcmp); the flat path keeps main's empty-list-is-true rule.RED → GREEN
Before the fix, all four new tests fail with
row_count == 0, expected 1:cypher_exec_where_not_on_relationship_target,cypher_exec_where_mixed_alias_and,_or,_xor.After:
cypher208 passed;cypher+mcp526 passed, 4 skipped (the Windows-only skips).eval_expr_partialrecurses and is added toCBM_RECURSION_WHITELIST;scripts/check-nolint-whitelist.shpasses.Formatting note: the whitelist macro was repacked by Homebrew clang-format 22 after the insertion lengthened a line.
AlignEscapedNewlinesand column packing are version-stable, but CI's clang-format 20 is the judge — if lint flags it, that is the only place to look.