From 937ad47e55706e086777a8690008bb202199c883 Mon Sep 17 00:00:00 2001 From: lorenzozanee Date: Sat, 3 Oct 2026 13:50:43 +0800 Subject: [PATCH 1/3] fix(cypher): defer WHERE until referenced bindings exist Signed-off-by: lorenzozanee --- src/cypher/cypher.c | 52 +++++++++++++++++++++++++- tests/test_cypher.c | 89 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 3cc31f9d76..2a18db7e84 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -4145,7 +4145,54 @@ static void execute_with_simple(cbm_return_clause_t *wc, binding_t *bindings, in } } -/* Apply post-WITH WHERE filter */ +static bool condition_bindings_available(const cbm_condition_t *condition, binding_t *binding) { + if (condition->variable && !binding_get(binding, condition->variable) && + !binding_get_edge(binding, condition->variable)) { + return false; + } + for (int i = 0; i < condition->arg_count; i++) { + const char *var = condition->args[i].variable; + if (var && !binding_get(binding, var) && !binding_get_edge(binding, var)) { + return false; + } + } + return true; +} + +static bool expr_bindings_available(const cbm_expr_t *expr, binding_t *binding) { + if (!expr) { + return true; + } + switch (expr->type) { + case EXPR_CONDITION: + return condition_bindings_available(&expr->cond, binding); + case EXPR_AND: + case EXPR_OR: + case EXPR_XOR: + return expr_bindings_available(expr->left, binding) && + expr_bindings_available(expr->right, binding); + case EXPR_NOT: + return expr_bindings_available(expr->left, binding); + } + return false; +} + +static bool where_bindings_available(const cbm_where_clause_t *where, binding_t *binding) { + if (!where) { + return true; + } + if (where->root) { + return expr_bindings_available(where->root, binding); + } + for (int i = 0; i < where->count; i++) { + if (!condition_bindings_available(&where->conditions[i], binding)) { + return false; + } + } + return true; +} + +/* Apply a WHERE filter after its referenced variables have been bound. */ static void filter_bindings_where(const cbm_where_clause_t *where, binding_t *vbindings, int *vcount) { int kept = 0; @@ -5149,7 +5196,8 @@ static int execute_single(cbm_store_t *store, cbm_query_t *q, const char *projec binding_t b = {0}; b.store = store; binding_set(&b, var_name, &scanned[i]); - bool pass = !q->where || eval_where(q->where, &b); + bool pass = !q->where || !where_bindings_available(q->where, &b) || + eval_where(q->where, &b); if (pass) { bindings[bind_count++] = b; } else { diff --git a/tests/test_cypher.c b/tests/test_cypher.c index abd3346831..b8e4ddb1fc 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -1757,6 +1757,32 @@ TEST(cypher_exists_has_outgoing_calls) { ASSERT_EQ(rc, 0); ASSERT_EQ(r.row_count, 2); cbm_cypher_result_free(&r); + + rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE b.name <> \"SubmitOrder\" " + "RETURN a.name, b.name", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 2); + cbm_cypher_result_free(&r); + + rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE b.name = \"SubmitOrder\" OR " + "NOT b.name = \"SubmitOrder\" RETURN count(*)", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "3"); + cbm_cypher_result_free(&r); + + rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WITH a, b WHERE NOT b.name = " + "\"SubmitOrder\" RETURN a.name, b.name", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 2); + cbm_cypher_result_free(&r); + cbm_store_close(s); PASS(); } @@ -3150,6 +3176,65 @@ TEST(cypher_exec_where_not) { PASS(); } +TEST(cypher_exec_where_not_target_node) { + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE NOT b.name = \"SubmitOrder\" " + "RETURN a.name, b.name", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 2); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + +TEST(cypher_exec_where_not_target_relationship) { + cbm_store_t *s = setup_cypher_multi_edge_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (a)-[r:HTTP_CALLS]->(b) WHERE NOT r.url_path = " + "\"/health\" RETURN r.url_path", + "testproj", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "/api/orders"); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + +TEST(cypher_exec_where_not_source_node) { + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE NOT a.name = " + "\"HandleOrder\" RETURN a.name, b.name", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "ValidateOrder"); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + +TEST(cypher_exec_where_not_target_node_count) { + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE NOT b.name = " + "\"SubmitOrder\" RETURN count(*)", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "2"); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + TEST(cypher_exec_where_in) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; @@ -5005,6 +5090,10 @@ SUITE(cypher) { RUN_TEST(cypher_exec_where_neq_bang); RUN_TEST(cypher_exec_where_ends_with); RUN_TEST(cypher_exec_where_not); + RUN_TEST(cypher_exec_where_not_target_node); + RUN_TEST(cypher_exec_where_not_target_relationship); + RUN_TEST(cypher_exec_where_not_source_node); + RUN_TEST(cypher_exec_where_not_target_node_count); RUN_TEST(cypher_exec_where_in); RUN_TEST(cypher_exec_where_not_in); RUN_TEST(cypher_exec_where_is_null); From 43a7e592027d3f4282cdb0314d92c142fca0165a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 19:23:03 +0200 Subject: [PATCH 2/3] revert(cypher): drop the early-WHERE deferral that #2253 superseded 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 (#2250). This branch fixed that by skipping the early pass until every alias the WHERE reads was bound. While it waited for review, #2253 (8ab555ee) 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 --- src/cypher/cypher.c | 52 ++------------------------------------------- 1 file changed, 2 insertions(+), 50 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 2a18db7e84..3cc31f9d76 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -4145,54 +4145,7 @@ static void execute_with_simple(cbm_return_clause_t *wc, binding_t *bindings, in } } -static bool condition_bindings_available(const cbm_condition_t *condition, binding_t *binding) { - if (condition->variable && !binding_get(binding, condition->variable) && - !binding_get_edge(binding, condition->variable)) { - return false; - } - for (int i = 0; i < condition->arg_count; i++) { - const char *var = condition->args[i].variable; - if (var && !binding_get(binding, var) && !binding_get_edge(binding, var)) { - return false; - } - } - return true; -} - -static bool expr_bindings_available(const cbm_expr_t *expr, binding_t *binding) { - if (!expr) { - return true; - } - switch (expr->type) { - case EXPR_CONDITION: - return condition_bindings_available(&expr->cond, binding); - case EXPR_AND: - case EXPR_OR: - case EXPR_XOR: - return expr_bindings_available(expr->left, binding) && - expr_bindings_available(expr->right, binding); - case EXPR_NOT: - return expr_bindings_available(expr->left, binding); - } - return false; -} - -static bool where_bindings_available(const cbm_where_clause_t *where, binding_t *binding) { - if (!where) { - return true; - } - if (where->root) { - return expr_bindings_available(where->root, binding); - } - for (int i = 0; i < where->count; i++) { - if (!condition_bindings_available(&where->conditions[i], binding)) { - return false; - } - } - return true; -} - -/* Apply a WHERE filter after its referenced variables have been bound. */ +/* Apply post-WITH WHERE filter */ static void filter_bindings_where(const cbm_where_clause_t *where, binding_t *vbindings, int *vcount) { int kept = 0; @@ -5196,8 +5149,7 @@ static int execute_single(cbm_store_t *store, cbm_query_t *q, const char *projec binding_t b = {0}; b.store = store; binding_set(&b, var_name, &scanned[i]); - bool pass = !q->where || !where_bindings_available(q->where, &b) || - eval_where(q->where, &b); + bool pass = !q->where || eval_where(q->where, &b); if (pass) { bindings[bind_count++] = b; } else { From 6d2d30c3ea2191adfc000f57d31a95d14b7b290b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 19:23:17 +0200 Subject: [PATCH 3/3] fix(cypher): the early WHERE pass waits for every alias a condition reads eval_condition_partial (from #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, ) 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 Signed-off-by: Martin Vogel --- src/cypher/cypher.c | 17 ++++++++++++++--- tests/test_cypher.c | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 8eaae7a268..c86be43bfc 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -2899,12 +2899,23 @@ static cyp_partial_t partial_or(cyp_partial_t l, cyp_partial_t r) { : CYP_PARTIAL_UNKNOWN; } -/* A condition whose alias is not bound yet is UNKNOWN; a literal-only LHS - * (func condition with no variable arg) has nothing to wait for. */ +static bool partial_alias_unbound(binding_t *b, const char *alias) { + return alias && !binding_get(b, alias) && !binding_get_edge(b, alias); +} + +/* A condition is UNKNOWN while any alias it reads is not bound yet: its own + * variable, and every function argument's -- a func condition is filed under + * its first argument's alias, so the b in coalesce(a.x, b.name) is only seen + * here. A literal-only LHS has nothing to wait for. */ static cyp_partial_t eval_condition_partial(const cbm_condition_t *c, binding_t *b) { - if (c->variable && !binding_get(b, c->variable) && !binding_get_edge(b, c->variable)) { + if (partial_alias_unbound(b, c->variable)) { return CYP_PARTIAL_UNKNOWN; } + for (int i = 0; i < c->arg_count; i++) { + if (partial_alias_unbound(b, c->args[i].variable)) { + return CYP_PARTIAL_UNKNOWN; + } + } return eval_condition(c, b) ? CYP_PARTIAL_TRUE : CYP_PARTIAL_FALSE; } diff --git a/tests/test_cypher.c b/tests/test_cypher.c index 06c3131da0..ec3afbfc28 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -3463,6 +3463,27 @@ TEST(cypher_exec_where_not_target_node_count) { PASS(); } +/* The early WHERE pass must wait for every alias a condition reads. A function + * condition is filed under its first argument's alias, so in + * coalesce(a.no_such_prop, b.name) the target b hid behind the bound a: the + * seed row read coalesce(NULL, ) as a definite mismatch and pruned + * every row before CALLS expansion, while the same predicate after WITH was + * right. */ +TEST(cypher_exec_where_func_arg_on_unbound_target) { + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (a)-[:CALLS]->(b) WHERE coalesce(a.no_such_prop, b.name) = " + "\"SubmitOrder\" RETURN a.name", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "ValidateOrder"); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + TEST(cypher_exec_where_in) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; @@ -6332,6 +6353,7 @@ SUITE(cypher) { RUN_TEST(cypher_exec_where_not_target_relationship); RUN_TEST(cypher_exec_where_not_source_node); RUN_TEST(cypher_exec_where_not_target_node_count); + RUN_TEST(cypher_exec_where_func_arg_on_unbound_target); RUN_TEST(cypher_exec_where_in); RUN_TEST(cypher_exec_where_not_in); RUN_TEST(cypher_exec_where_is_null);