Repository navigation
Conversation
Signed-off-by: Antonio Espinosa <antespi@gmail.com>
antespi
marked this pull request as ready for review
October 2, 2026 17:57
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Author
|
Already fixed in #2253 |
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.
What does this PR do?
WHERE <condition on b> AND NOT <condition on a>returned 0 rows, and the response still reported the empty result as exact (total_relation: eq).NOTon its own,OR NOTand<>all worked. OnlyNOTas an operand ofANDfailed, with or without parentheses.Root cause
cypher_plan_seed_from_selective_endswaps a single-hop pattern so the scan starts from the end that has an equality onname. In the query above, that isb.execute_singlethen runs an early WHERE on each scanned node. At that point only the seed node is bound, soais still unbound.eval_conditionreturnstruefor an unbound variable (if (!actual) return true;).EXPR_NOTflips that tofalse, so every row is dropped before the pattern is expanded. The late WHERE never gets to see them.HAS_LABELhas the same problem:WHERE b.name = 'x' AND a:Functionwas also pruned to 0, because the label test on an unbound node returnsfalse.Without the swap (no equality on the far node), the near node is the one bound, which is why
NOTalone andOR NOTworked.Fix
The early WHERE now uses three-valued (Kleene) logic, in
eval_expr_tri/eval_where_early:The late WHERE is unchanged and still decides with every variable bound. Pruning stays sound and loses nothing compared with before: a condition on an unbound variable never pruned a row unless something negated it.
eval_expr_triis recursive likeeval_expr, so it is added toCBM_RECURSION_WHITELIST.Tests
New test
cypher_exec_where_and_not_on_unbound_seed_sideintests/test_cypher.c. Each query starts from the far node:b = X AND NOT (a.file_path CONTAINS ...)b = X AND NOT a.name = ...NOT (...) AND b = X(operand order swapped)b = X AND NOT (a.p = ... AND a.q CONTAINS ...)(b = X AND NOT a.name = ...) OR b = Y(NOT inside AND inside OR)b = X AND a:Function(label test on the unbound side)Results:
cyphersuite passes (206) and so doesmcp(320 passed, 4 skipped).lint-no-suppresspasses, and the changed files passclang-format --dry-run --Werror.I also checked a real index (a NestJS monorepo, about 64k nodes) with
WHERE b.name = 'BillableStatusChangedEvent' AND NOT (a.file_path CONTAINS '.spec.'). It returned 0 rows before the fix and returns the one non-spec caller after it.Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test): only thecypherandmcpsuites were run on this branchmake -f Makefile.cbm lint-ci): onlylint-no-suppressand clang-format on the changed files were run🤖 Generated with Claude Code