Skip to content

Commit 4382b97

Browse files
DeusDataDeusData
committed
feat(cypher): coalesce(var.prop, literal) in WHERE clauses
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 DeusData#874 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent ae95649 commit 4382b97

3 files changed

Lines changed: 166 additions & 50 deletions

File tree

‎src/cypher/cypher.c‎

Lines changed: 96 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ static void expr_free(cbm_expr_t *e) {
755755
safe_str_free(&cur->cond.property);
756756
safe_str_free(&cur->cond.op);
757757
safe_str_free(&cur->cond.value);
758+
safe_str_free(&cur->cond.coalesce_default);
758759
for (int i = 0; i < cur->cond.in_value_count; i++) {
759760
safe_str_free(&cur->cond.in_values[i]);
760761
}
@@ -1007,6 +1008,61 @@ static cbm_expr_t *parse_exists_predicate(parser_t *p, bool negated) {
10071008
return expr_leaf(c);
10081009
}
10091010

1011+
static bool cyp_ci_eq(const char *a, const char *b);
1012+
1013+
/* Parse the operator + value tail shared by every condition subject
1014+
* (var[.prop] and coalesce(var.prop, literal)): IS [NOT] NULL, IN [...],
1015+
* or a comparison operator with a literal value. Consumes/frees *c. */
1016+
static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
1017+
/* IS NULL / IS NOT NULL */
1018+
if (check(p, TOK_IS)) {
1019+
advance(p);
1020+
if (match(p, TOK_NOT)) {
1021+
c->op = heap_strdup("IS NOT NULL");
1022+
expect(p, TOK_NULL_KW);
1023+
} else {
1024+
expect(p, TOK_NULL_KW);
1025+
c->op = heap_strdup("IS NULL");
1026+
}
1027+
return expr_leaf(*c);
1028+
}
1029+
1030+
/* IN [...] */
1031+
if (check(p, TOK_IN)) {
1032+
return parse_in_list(p, c);
1033+
}
1034+
1035+
/* Standard operators */
1036+
c->op = parse_comparison_op(p);
1037+
if (!c->op) {
1038+
snprintf(p->error, sizeof(p->error), "unexpected operator at pos %d", peek(p)->pos);
1039+
safe_str_free(&c->variable);
1040+
safe_str_free(&c->property);
1041+
safe_str_free(&c->coalesce_default);
1042+
return NULL;
1043+
}
1044+
1045+
/* Value */
1046+
if (check(p, TOK_STRING) || check(p, TOK_NUMBER)) {
1047+
c->value = heap_strdup(advance(p)->text);
1048+
} else if (check(p, TOK_TRUE)) {
1049+
advance(p);
1050+
c->value = heap_strdup("true");
1051+
} else if (check(p, TOK_FALSE)) {
1052+
advance(p);
1053+
c->value = heap_strdup("false");
1054+
} else {
1055+
snprintf(p->error, sizeof(p->error), "expected value at pos %d", peek(p)->pos);
1056+
safe_str_free(&c->variable);
1057+
safe_str_free(&c->property);
1058+
safe_str_free(&c->op);
1059+
safe_str_free(&c->coalesce_default);
1060+
return NULL;
1061+
}
1062+
1063+
return expr_leaf(*c);
1064+
}
1065+
10101066
static cbm_expr_t *parse_condition_expr(parser_t *p) {
10111067
/* Check for NOT prefix at condition level (e.g. NOT n.name CONTAINS "x") */
10121068
bool negated = match(p, TOK_NOT);
@@ -1016,6 +1072,40 @@ static cbm_expr_t *parse_condition_expr(parser_t *p) {
10161072
return parse_exists_predicate(p, negated);
10171073
}
10181074

1075+
/* coalesce(var.prop, <literal>) as a null-safe condition subject (#874).
1076+
* The default literal substitutes a missing/empty property at eval time;
1077+
* any comparison operator may follow, exactly as with a bare var.prop. */
1078+
if (check(p, TOK_IDENT) && cyp_ci_eq(peek(p)->text, "coalesce") &&
1079+
p->pos + SKIP_ONE < p->count && p->tokens[p->pos + SKIP_ONE].type == TOK_LPAREN) {
1080+
advance(p); /* coalesce */
1081+
advance(p); /* ( */
1082+
const cbm_token_t *cvar = expect(p, TOK_IDENT);
1083+
if (!cvar || !expect(p, TOK_DOT)) {
1084+
return NULL;
1085+
}
1086+
const cbm_token_t *cprop = expect(p, TOK_IDENT);
1087+
if (!cprop || !expect(p, TOK_COMMA)) {
1088+
return NULL;
1089+
}
1090+
if (!check(p, TOK_STRING) && !check(p, TOK_NUMBER)) {
1091+
return NULL; /* supported form: coalesce(var.prop, literal) */
1092+
}
1093+
const cbm_token_t *cdef = peek(p);
1094+
cbm_condition_t cc = {0};
1095+
cc.negated = negated;
1096+
cc.variable = heap_strdup(cvar->text);
1097+
cc.property = heap_strdup(cprop->text);
1098+
cc.coalesce_default = heap_strdup(cdef->text);
1099+
advance(p);
1100+
if (!expect(p, TOK_RPAREN)) {
1101+
safe_str_free(&cc.variable);
1102+
safe_str_free(&cc.property);
1103+
safe_str_free(&cc.coalesce_default);
1104+
return NULL;
1105+
}
1106+
return parse_condition_op(p, &cc);
1107+
}
1108+
10191109
const cbm_token_t *var = expect(p, TOK_IDENT);
10201110
if (!var) {
10211111
return NULL;
@@ -1051,51 +1141,7 @@ static cbm_expr_t *parse_condition_expr(parser_t *p) {
10511141
c.property = NULL;
10521142
}
10531143

1054-
/* IS NULL / IS NOT NULL */
1055-
if (check(p, TOK_IS)) {
1056-
advance(p);
1057-
if (match(p, TOK_NOT)) {
1058-
c.op = heap_strdup("IS NOT NULL");
1059-
expect(p, TOK_NULL_KW);
1060-
} else {
1061-
expect(p, TOK_NULL_KW);
1062-
c.op = heap_strdup("IS NULL");
1063-
}
1064-
return expr_leaf(c);
1065-
}
1066-
1067-
/* IN [...] */
1068-
if (check(p, TOK_IN)) {
1069-
return parse_in_list(p, &c);
1070-
}
1071-
1072-
/* Standard operators */
1073-
c.op = parse_comparison_op(p);
1074-
if (!c.op) {
1075-
snprintf(p->error, sizeof(p->error), "unexpected operator at pos %d", peek(p)->pos);
1076-
safe_str_free(&c.variable);
1077-
safe_str_free(&c.property);
1078-
return NULL;
1079-
}
1080-
1081-
/* Value */
1082-
if (check(p, TOK_STRING) || check(p, TOK_NUMBER)) {
1083-
c.value = heap_strdup(advance(p)->text);
1084-
} else if (check(p, TOK_TRUE)) {
1085-
advance(p);
1086-
c.value = heap_strdup("true");
1087-
} else if (check(p, TOK_FALSE)) {
1088-
advance(p);
1089-
c.value = heap_strdup("false");
1090-
} else {
1091-
snprintf(p->error, sizeof(p->error), "expected value at pos %d", peek(p)->pos);
1092-
safe_str_free(&c.variable);
1093-
safe_str_free(&c.property);
1094-
safe_str_free(&c.op);
1095-
return NULL;
1096-
}
1097-
1098-
return expr_leaf(c);
1144+
return parse_condition_op(p, &c);
10991145
}
11001146

11011147
/* Atom: ( expr ) | condition */
@@ -2444,6 +2490,11 @@ static bool eval_condition(const cbm_condition_t *c, binding_t *b) {
24442490
}
24452491

24462492
const char *actual = resolve_condition_value(c, b);
2493+
/* coalesce(var.prop, literal) (#874): a missing/empty property value
2494+
* falls back to the literal default before the operator runs. */
2495+
if (c->coalesce_default && (!actual || actual[0] == '\0')) {
2496+
actual = c->coalesce_default;
2497+
}
24472498
if (!actual) {
24482499
return true;
24492500
}

‎src/cypher/cypher.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ typedef struct {
188188
const char *op; /* "=", "<>", "=~", "CONTAINS", "STARTS WITH", "ENDS WITH",
189189
">", "<", ">=", "<=", "IN", "IS NULL", "IS NOT NULL" */
190190
const char *value;
191-
bool negated; /* NOT prefix */
191+
bool negated; /* NOT prefix */
192+
/* coalesce(var.prop, literal) in WHERE (#874): when set, a missing/empty
193+
* property value is substituted with this literal before the op runs. */
194+
const char *coalesce_default;
192195
const char **in_values; /* IN [...] list */
193196
int in_value_count;
194197
/* EXISTS { (var)-[:value]->() } predicate (op=="EXISTS"): `variable` is the

‎tests/test_cypher.c‎

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,67 @@ TEST(cypher_exec_where_eq) {
503503
PASS();
504504
}
505505

506+
/* #874: coalesce(var.prop, literal) in WHERE — null-safe numeric filters
507+
* for audit queries over OPTIONAL graph properties. The parser rejected the
508+
* call outright ("unexpected operator"); RETURN-side coalesce already
509+
* worked, so only the WHERE leaf needs it. Semantics: when the property is
510+
* missing/empty, the literal default is compared instead. */
511+
TEST(cypher_exec_where_coalesce_issue874) {
512+
cbm_store_t *s = cbm_store_open_memory();
513+
cbm_store_upsert_project(s, "test", "/tmp/test");
514+
cbm_node_t a = {.project = "test",
515+
.label = "Function",
516+
.name = "deep_a",
517+
.qualified_name = "test.mod.deep_a",
518+
.file_path = "mod.py",
519+
.start_line = 1,
520+
.end_line = 2,
521+
.properties_json = "{\"transitive_loop_depth\":3}"};
522+
cbm_node_t b = {.project = "test",
523+
.label = "Function",
524+
.name = "deep_b",
525+
.qualified_name = "test.mod.deep_b",
526+
.file_path = "mod.py",
527+
.start_line = 3,
528+
.end_line = 4,
529+
.properties_json = "{\"transitive_loop_depth\":1}"};
530+
cbm_node_t c = {.project = "test",
531+
.label = "Function",
532+
.name = "plain_c",
533+
.qualified_name = "test.mod.plain_c",
534+
.file_path = "mod.py",
535+
.start_line = 5,
536+
.end_line = 6};
537+
ASSERT_GT(cbm_store_upsert_node(s, &a), 0);
538+
ASSERT_GT(cbm_store_upsert_node(s, &b), 0);
539+
ASSERT_GT(cbm_store_upsert_node(s, &c), 0);
540+
541+
/* Default FAILS the predicate: only the node with depth 3 matches. */
542+
cbm_cypher_result_t r = {0};
543+
int rc = cbm_cypher_execute(s,
544+
"MATCH (f:Function) WHERE "
545+
"coalesce(f.transitive_loop_depth, 0) >= 2 "
546+
"RETURN f.qualified_name LIMIT 10",
547+
"test", 0, &r);
548+
ASSERT_EQ(rc, 0);
549+
ASSERT_EQ(r.row_count, 1);
550+
cbm_cypher_result_free(&r);
551+
552+
/* Default PASSES: the property-less node is included via the default. */
553+
cbm_cypher_result_t r2 = {0};
554+
rc = cbm_cypher_execute(s,
555+
"MATCH (f:Function) WHERE "
556+
"coalesce(f.transitive_loop_depth, 9) >= 2 "
557+
"RETURN f.qualified_name LIMIT 10",
558+
"test", 0, &r2);
559+
ASSERT_EQ(rc, 0);
560+
ASSERT_EQ(r2.row_count, 2); /* deep_a (3) + plain_c (default 9) */
561+
cbm_cypher_result_free(&r2);
562+
563+
cbm_store_close(s);
564+
PASS();
565+
}
566+
506567
TEST(cypher_exec_where_regex) {
507568
cbm_store_t *s = setup_cypher_store();
508569
cbm_cypher_result_t r = {0};
@@ -2548,8 +2609,8 @@ TEST(cypher_issue873_distinct_order_limit_dedupes_before_limit) {
25482609
TEST(cypher_issue873_distinct_limit_dedupes_before_limit) {
25492610
cbm_store_t *s = setup_cypher_store();
25502611
cbm_cypher_result_t r = {0};
2551-
int rc = cbm_cypher_execute(
2552-
s, "MATCH (n) RETURN DISTINCT n.label AS label LIMIT 2", "test", 0, &r);
2612+
int rc =
2613+
cbm_cypher_execute(s, "MATCH (n) RETURN DISTINCT n.label AS label LIMIT 2", "test", 0, &r);
25532614
ASSERT_EQ(rc, 0);
25542615
ASSERT_NULL(r.error);
25552616
ASSERT_EQ(r.row_count, 2);
@@ -2563,8 +2624,8 @@ TEST(cypher_issue873_distinct_order_skip_limit_dedupes_before_skip) {
25632624
cbm_store_t *s = setup_cypher_store();
25642625
cbm_cypher_result_t r = {0};
25652626
int rc = cbm_cypher_execute(
2566-
s, "MATCH (n) RETURN DISTINCT n.label AS label ORDER BY label SKIP 1 LIMIT 1", "test",
2567-
0, &r);
2627+
s, "MATCH (n) RETURN DISTINCT n.label AS label ORDER BY label SKIP 1 LIMIT 1", "test", 0,
2628+
&r);
25682629
ASSERT_EQ(rc, 0);
25692630
ASSERT_NULL(r.error);
25702631
ASSERT_EQ(r.row_count, 1);
@@ -2721,6 +2782,7 @@ SUITE(cypher) {
27212782
RUN_TEST(cypher_issue252_tointeger);
27222783
RUN_TEST(cypher_issue305_count_star_alias);
27232784
RUN_TEST(cypher_exec_where_eq);
2785+
RUN_TEST(cypher_exec_where_coalesce_issue874);
27242786
RUN_TEST(cypher_exec_where_regex);
27252787
RUN_TEST(cypher_exec_where_contains);
27262788
RUN_TEST(cypher_exec_where_starts_with);

0 commit comments

Comments
 (0)