Merge pull request #1034 from DeusData/fix/874-cypher-coalesce

feat(cypher): coalesce(var.prop, literal) in WHERE clauses

Author: DeusData

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, <literal>) 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;
     }

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

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);