From 4382b97908a6d3da562f85d890a0dc5ab857d5af Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 11 Jul 2026 19:10:07 +0200 Subject: [PATCH] feat(cypher): coalesce(var.prop, literal) in WHERE clauses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WHERE parser rejected coalesce() outright ('unexpected operator'), blocking null-safe numeric filters over optional graph properties — the exact audit-query shape from the report: MATCH (f:Function) WHERE coalesce(f.transitive_loop_depth, 0) >= 2 RETURN-side coalesce already worked; only the WHERE leaf lacked it. Implementation: the condition leaf gains a coalesce_default literal. The operator/value tail of condition parsing is factored into a shared parse_condition_op so both subjects (var[.prop] and coalesce(var.prop, literal)) accept the full operator set (comparisons, IS [NOT] NULL, IN). At eval time a missing/empty property value is substituted with the default before the operator runs. Supported form is coalesce(var.prop, literal) — matching openCypher's two-argument null-substitution for this subset. Reproduce-first: cypher_exec_where_coalesce_issue874 runs the reporter's exact query — RED (rc=-1 parse error) on the old parser — and pins both directions: a failing default excludes property-less nodes, a passing default includes them. Closes #874 Signed-off-by: Martin Vogel --- src/cypher/cypher.c | 141 ++++++++++++++++++++++++++++++-------------- src/cypher/cypher.h | 5 +- tests/test_cypher.c | 70 ++++++++++++++++++++-- 3 files changed, 166 insertions(+), 50 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 2127b2259..20fd051d1 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -755,6 +755,7 @@ static void expr_free(cbm_expr_t *e) { safe_str_free(&cur->cond.property); safe_str_free(&cur->cond.op); safe_str_free(&cur->cond.value); + safe_str_free(&cur->cond.coalesce_default); for (int i = 0; i < cur->cond.in_value_count; i++) { safe_str_free(&cur->cond.in_values[i]); } @@ -1007,6 +1008,61 @@ static cbm_expr_t *parse_exists_predicate(parser_t *p, bool negated) { return expr_leaf(c); } +static bool cyp_ci_eq(const char *a, const char *b); + +/* Parse the operator + value tail shared by every condition subject + * (var[.prop] and coalesce(var.prop, literal)): IS [NOT] NULL, IN [...], + * or a comparison operator with a literal value. Consumes/frees *c. */ +static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) { + /* IS NULL / IS NOT NULL */ + if (check(p, TOK_IS)) { + advance(p); + if (match(p, TOK_NOT)) { + c->op = heap_strdup("IS NOT NULL"); + expect(p, TOK_NULL_KW); + } else { + expect(p, TOK_NULL_KW); + c->op = heap_strdup("IS NULL"); + } + return expr_leaf(*c); + } + + /* IN [...] */ + if (check(p, TOK_IN)) { + return parse_in_list(p, c); + } + + /* Standard operators */ + c->op = parse_comparison_op(p); + if (!c->op) { + snprintf(p->error, sizeof(p->error), "unexpected operator at pos %d", peek(p)->pos); + safe_str_free(&c->variable); + safe_str_free(&c->property); + safe_str_free(&c->coalesce_default); + return NULL; + } + + /* Value */ + if (check(p, TOK_STRING) || check(p, TOK_NUMBER)) { + c->value = heap_strdup(advance(p)->text); + } else if (check(p, TOK_TRUE)) { + advance(p); + c->value = heap_strdup("true"); + } else if (check(p, TOK_FALSE)) { + advance(p); + c->value = heap_strdup("false"); + } else { + snprintf(p->error, sizeof(p->error), "expected value at pos %d", peek(p)->pos); + safe_str_free(&c->variable); + safe_str_free(&c->property); + safe_str_free(&c->op); + safe_str_free(&c->coalesce_default); + return NULL; + } + + return expr_leaf(*c); +} + static cbm_expr_t *parse_condition_expr(parser_t *p) { /* Check for NOT prefix at condition level (e.g. NOT n.name CONTAINS "x") */ bool negated = match(p, TOK_NOT); @@ -1016,6 +1072,40 @@ static cbm_expr_t *parse_condition_expr(parser_t *p) { return parse_exists_predicate(p, negated); } + /* coalesce(var.prop, ) as a null-safe condition subject (#874). + * The default literal substitutes a missing/empty property at eval time; + * any comparison operator may follow, exactly as with a bare var.prop. */ + if (check(p, TOK_IDENT) && cyp_ci_eq(peek(p)->text, "coalesce") && + p->pos + SKIP_ONE < p->count && p->tokens[p->pos + SKIP_ONE].type == TOK_LPAREN) { + advance(p); /* coalesce */ + advance(p); /* ( */ + const cbm_token_t *cvar = expect(p, TOK_IDENT); + if (!cvar || !expect(p, TOK_DOT)) { + return NULL; + } + const cbm_token_t *cprop = expect(p, TOK_IDENT); + if (!cprop || !expect(p, TOK_COMMA)) { + return NULL; + } + if (!check(p, TOK_STRING) && !check(p, TOK_NUMBER)) { + return NULL; /* supported form: coalesce(var.prop, literal) */ + } + const cbm_token_t *cdef = peek(p); + cbm_condition_t cc = {0}; + cc.negated = negated; + cc.variable = heap_strdup(cvar->text); + cc.property = heap_strdup(cprop->text); + cc.coalesce_default = heap_strdup(cdef->text); + advance(p); + if (!expect(p, TOK_RPAREN)) { + safe_str_free(&cc.variable); + safe_str_free(&cc.property); + safe_str_free(&cc.coalesce_default); + return NULL; + } + return parse_condition_op(p, &cc); + } + const cbm_token_t *var = expect(p, TOK_IDENT); if (!var) { return NULL; @@ -1051,51 +1141,7 @@ static cbm_expr_t *parse_condition_expr(parser_t *p) { c.property = NULL; } - /* IS NULL / IS NOT NULL */ - if (check(p, TOK_IS)) { - advance(p); - if (match(p, TOK_NOT)) { - c.op = heap_strdup("IS NOT NULL"); - expect(p, TOK_NULL_KW); - } else { - expect(p, TOK_NULL_KW); - c.op = heap_strdup("IS NULL"); - } - return expr_leaf(c); - } - - /* IN [...] */ - if (check(p, TOK_IN)) { - return parse_in_list(p, &c); - } - - /* Standard operators */ - c.op = parse_comparison_op(p); - if (!c.op) { - snprintf(p->error, sizeof(p->error), "unexpected operator at pos %d", peek(p)->pos); - safe_str_free(&c.variable); - safe_str_free(&c.property); - return NULL; - } - - /* Value */ - if (check(p, TOK_STRING) || check(p, TOK_NUMBER)) { - c.value = heap_strdup(advance(p)->text); - } else if (check(p, TOK_TRUE)) { - advance(p); - c.value = heap_strdup("true"); - } else if (check(p, TOK_FALSE)) { - advance(p); - c.value = heap_strdup("false"); - } else { - snprintf(p->error, sizeof(p->error), "expected value at pos %d", peek(p)->pos); - safe_str_free(&c.variable); - safe_str_free(&c.property); - safe_str_free(&c.op); - return NULL; - } - - return expr_leaf(c); + return parse_condition_op(p, &c); } /* Atom: ( expr ) | condition */ @@ -2444,6 +2490,11 @@ static bool eval_condition(const cbm_condition_t *c, binding_t *b) { } const char *actual = resolve_condition_value(c, b); + /* coalesce(var.prop, literal) (#874): a missing/empty property value + * falls back to the literal default before the operator runs. */ + if (c->coalesce_default && (!actual || actual[0] == '\0')) { + actual = c->coalesce_default; + } if (!actual) { return true; } diff --git a/src/cypher/cypher.h b/src/cypher/cypher.h index 5966c2076..c8452bd56 100644 --- a/src/cypher/cypher.h +++ b/src/cypher/cypher.h @@ -188,7 +188,10 @@ typedef struct { const char *op; /* "=", "<>", "=~", "CONTAINS", "STARTS WITH", "ENDS WITH", ">", "<", ">=", "<=", "IN", "IS NULL", "IS NOT NULL" */ const char *value; - bool negated; /* NOT prefix */ + bool negated; /* NOT prefix */ + /* coalesce(var.prop, literal) in WHERE (#874): when set, a missing/empty + * property value is substituted with this literal before the op runs. */ + const char *coalesce_default; const char **in_values; /* IN [...] list */ int in_value_count; /* EXISTS { (var)-[:value]->() } predicate (op=="EXISTS"): `variable` is the diff --git a/tests/test_cypher.c b/tests/test_cypher.c index 929e4d6a9..b618876fa 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -503,6 +503,67 @@ TEST(cypher_exec_where_eq) { PASS(); } +/* #874: coalesce(var.prop, literal) in WHERE — null-safe numeric filters + * for audit queries over OPTIONAL graph properties. The parser rejected the + * call outright ("unexpected operator"); RETURN-side coalesce already + * worked, so only the WHERE leaf needs it. Semantics: when the property is + * missing/empty, the literal default is compared instead. */ +TEST(cypher_exec_where_coalesce_issue874) { + cbm_store_t *s = cbm_store_open_memory(); + cbm_store_upsert_project(s, "test", "/tmp/test"); + cbm_node_t a = {.project = "test", + .label = "Function", + .name = "deep_a", + .qualified_name = "test.mod.deep_a", + .file_path = "mod.py", + .start_line = 1, + .end_line = 2, + .properties_json = "{\"transitive_loop_depth\":3}"}; + cbm_node_t b = {.project = "test", + .label = "Function", + .name = "deep_b", + .qualified_name = "test.mod.deep_b", + .file_path = "mod.py", + .start_line = 3, + .end_line = 4, + .properties_json = "{\"transitive_loop_depth\":1}"}; + cbm_node_t c = {.project = "test", + .label = "Function", + .name = "plain_c", + .qualified_name = "test.mod.plain_c", + .file_path = "mod.py", + .start_line = 5, + .end_line = 6}; + ASSERT_GT(cbm_store_upsert_node(s, &a), 0); + ASSERT_GT(cbm_store_upsert_node(s, &b), 0); + ASSERT_GT(cbm_store_upsert_node(s, &c), 0); + + /* Default FAILS the predicate: only the node with depth 3 matches. */ + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (f:Function) WHERE " + "coalesce(f.transitive_loop_depth, 0) >= 2 " + "RETURN f.qualified_name LIMIT 10", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + cbm_cypher_result_free(&r); + + /* Default PASSES: the property-less node is included via the default. */ + cbm_cypher_result_t r2 = {0}; + rc = cbm_cypher_execute(s, + "MATCH (f:Function) WHERE " + "coalesce(f.transitive_loop_depth, 9) >= 2 " + "RETURN f.qualified_name LIMIT 10", + "test", 0, &r2); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r2.row_count, 2); /* deep_a (3) + plain_c (default 9) */ + cbm_cypher_result_free(&r2); + + cbm_store_close(s); + PASS(); +} + TEST(cypher_exec_where_regex) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; @@ -2548,8 +2609,8 @@ TEST(cypher_issue873_distinct_order_limit_dedupes_before_limit) { TEST(cypher_issue873_distinct_limit_dedupes_before_limit) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; - int rc = cbm_cypher_execute( - s, "MATCH (n) RETURN DISTINCT n.label AS label LIMIT 2", "test", 0, &r); + int rc = + cbm_cypher_execute(s, "MATCH (n) RETURN DISTINCT n.label AS label LIMIT 2", "test", 0, &r); ASSERT_EQ(rc, 0); ASSERT_NULL(r.error); ASSERT_EQ(r.row_count, 2); @@ -2563,8 +2624,8 @@ TEST(cypher_issue873_distinct_order_skip_limit_dedupes_before_skip) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; int rc = cbm_cypher_execute( - s, "MATCH (n) RETURN DISTINCT n.label AS label ORDER BY label SKIP 1 LIMIT 1", "test", - 0, &r); + s, "MATCH (n) RETURN DISTINCT n.label AS label ORDER BY label SKIP 1 LIMIT 1", "test", 0, + &r); ASSERT_EQ(rc, 0); ASSERT_NULL(r.error); ASSERT_EQ(r.row_count, 1); @@ -2721,6 +2782,7 @@ SUITE(cypher) { RUN_TEST(cypher_issue252_tointeger); RUN_TEST(cypher_issue305_count_star_alias); RUN_TEST(cypher_exec_where_eq); + RUN_TEST(cypher_exec_where_coalesce_issue874); RUN_TEST(cypher_exec_where_regex); RUN_TEST(cypher_exec_where_contains); RUN_TEST(cypher_exec_where_starts_with);