fix(cypher): support coalesce() and other multi-arg scalar functions in WHERE (#874)

Signed-off-by: Harshita Joshi <j.harshitaa06@gmail.com>

src/cypher/cypher.c

@@ -760,6 +760,13 @@ static void expr_free(cbm_expr_t *e) {
                 safe_str_free(&cur->cond.in_values[i]);
             }
             free(cur->cond.in_values);
+            safe_str_free(&cur->cond.func);
+            for (int i = 0; i < cur->cond.arg_count; i++) {
+                safe_str_free(&cur->cond.args[i].variable);
+                safe_str_free(&cur->cond.args[i].property);
+                safe_str_free(&cur->cond.args[i].literal);
+            }
+            free(cur->cond.args);
         }
         if (cur->right) {
             if (top < EXPR_FREE_STACK) {
@@ -838,6 +845,37 @@ static const char *unsupported_clause_error(cbm_token_type_t type) {
 
 /* Forward declarations for recursive descent */
 static cbm_expr_t *parse_or_expr(parser_t *p);
+/* Multi-arg scalar function support, shared with the RETURN-item parser (#874) */
+static bool is_multiarg_func_call(parser_t *p);
+static int parse_multiarg_func_item(parser_t *p, cbm_return_item_t *item);
+
+/* Free a multi-arg function argument array. */
+static void func_args_free(cbm_func_arg_t *args, int count) {
+    for (int i = 0; i < count; i++) {
+        safe_str_free(&args[i].variable);
+        safe_str_free(&args[i].property);
+        safe_str_free(&args[i].literal);
+    }
+    free(args);
+}
+
+/* Free the fields of a partially-parsed multi-arg function item. */
+static void func_item_fields_free(cbm_return_item_t *item) {
+    safe_str_free(&item->variable);
+    safe_str_free(&item->property);
+    safe_str_free(&item->func);
+    func_args_free(item->args, item->arg_count);
+    item->args = NULL;
+    item->arg_count = 0;
+}
+
+/* Free the function-call fields of a WHERE condition (#874). */
+static void cond_func_fields_free(cbm_condition_t *c) {
+    safe_str_free(&c->func);
+    func_args_free(c->args, c->arg_count);
+    c->args = NULL;
+    c->arg_count = 0;
+}
 
 /* Parse IN [val, val, ...] list. Returns expr_leaf or NULL on error. */
 static cbm_expr_t *parse_in_list(parser_t *p, cbm_condition_t *c) {
@@ -1008,11 +1046,9 @@ 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. */
+ * (var[.prop] and multi-arg functions like coalesce(...)): IS [NOT] NULL,
+ * IN [...], or a comparison operator with a literal value. */
 static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
     /* IS NULL / IS NOT NULL */
     if (check(p, TOK_IS)) {
@@ -1029,13 +1065,18 @@ static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
 
     /* IN [...] */
     if (check(p, TOK_IN)) {
-        return parse_in_list(p, c);
+        cbm_expr_t *e = parse_in_list(p, c);
+        if (!e) {
+            cond_func_fields_free(c);
+        }
+        return e;
     }
 
     /* 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);
+        cond_func_fields_free(c);
         safe_str_free(&c->variable);
         safe_str_free(&c->property);
         safe_str_free(&c->coalesce_default);
@@ -1053,6 +1094,7 @@ static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
         c->value = heap_strdup("false");
     } else {
         snprintf(p->error, sizeof(p->error), "expected value at pos %d", peek(p)->pos);
+        cond_func_fields_free(c);
         safe_str_free(&c->variable);
         safe_str_free(&c->property);
         safe_str_free(&c->op);
@@ -1063,6 +1105,75 @@ static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
     return expr_leaf(*c);
 }
 
+/* parse_condition_lhs result: the label-test form is a complete condition. */
+enum { COND_LHS_COMPLETE = 1 };
+
+/* Parse the left-hand side of a WHERE condition into c.
+ * Returns CBM_NOT_FOUND on error, 0 when an operator/value should follow, and
+ * COND_LHS_COMPLETE when the condition is already complete (label test). */
+static int parse_condition_lhs(parser_t *p, cbm_condition_t *c) {
+    if (is_multiarg_func_call(p)) {
+        /* Multi-arg scalar function LHS: coalesce(f.depth, 0) >= 2 (#874).
+         * Reuse the RETURN-item parser, then move ownership into the condition. */
+        cbm_return_item_t fitem;
+        memset(&fitem, 0, sizeof(fitem));
+        if (parse_multiarg_func_item(p, &fitem) < 0) {
+            func_item_fields_free(&fitem);
+            return CBM_NOT_FOUND;
+        }
+        c->variable = fitem.variable;
+        c->property = fitem.property;
+        c->func = fitem.func;
+        c->args = fitem.args;
+        c->arg_count = fitem.arg_count;
+        return 0;
+    }
+
+    if (check(p, TOK_IDENT) && p->pos + SKIP_ONE < p->count &&
+        p->tokens[p->pos + SKIP_ONE].type == TOK_LPAREN) {
+        /* Unrecognised function call in WHERE — fail loudly with the supported
+         * set instead of the misleading "unexpected operator" (#874). */
+        snprintf(p->error, sizeof(p->error),
+                 "unsupported function '%s' in WHERE (supported: coalesce, substring, replace, "
+                 "left, right)",
+                 peek(p)->text);
+        return CBM_NOT_FOUND;
+    }
+
+    const cbm_token_t *var = expect(p, TOK_IDENT);
+    if (!var) {
+        return CBM_NOT_FOUND;
+    }
+
+    /* Label test: WHERE n:Label (openCypher, #241). Modelled as a leaf with
+     * op="HAS_LABEL" and value=Label, evaluated against the bound node's label. */
+    if (check(p, TOK_COLON)) {
+        advance(p);
+        const cbm_token_t *lbl = expect(p, TOK_IDENT);
+        if (!lbl) {
+            return CBM_NOT_FOUND;
+        }
+        c->variable = heap_strdup(var->text);
+        c->op = heap_strdup("HAS_LABEL");
+        c->value = heap_strdup(lbl->text);
+        return COND_LHS_COMPLETE;
+    }
+
+    if (match(p, TOK_DOT)) {
+        const cbm_token_t *prop = expect(p, TOK_IDENT);
+        if (!prop) {
+            return CBM_NOT_FOUND;
+        }
+        c->variable = heap_strdup(var->text);
+        c->property = heap_strdup(prop->text);
+    } else {
+        /* No dot: bare alias (e.g. post-WITH variable like "cnt") */
+        c->variable = heap_strdup(var->text);
+        c->property = NULL;
+    }
+    return 0;
+}
+
 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);
@@ -1072,73 +1183,16 @@ 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;
-    }
-
     cbm_condition_t c = {0};
     c.negated = negated;
 
-    /* Label test: WHERE n:Label (openCypher, #241). Modelled as a leaf with
-     * op="HAS_LABEL" and value=Label, evaluated against the bound node's label. */
-    if (check(p, TOK_COLON)) {
-        advance(p);
-        const cbm_token_t *lbl = expect(p, TOK_IDENT);
-        if (!lbl) {
-            return NULL;
-        }
-        c.variable = heap_strdup(var->text);
-        c.op = heap_strdup("HAS_LABEL");
-        c.value = heap_strdup(lbl->text);
-        return expr_leaf(c);
+    int lhs_rc = parse_condition_lhs(p, &c);
+    if (lhs_rc < 0) {
+        return NULL;
     }
-
-    if (match(p, TOK_DOT)) {
-        const cbm_token_t *prop = expect(p, TOK_IDENT);
-        if (!prop) {
-            return NULL;
-        }
-        c.variable = heap_strdup(var->text);
-        c.property = heap_strdup(prop->text);
-    } else {
-        /* No dot: bare alias (e.g. post-WITH variable like "cnt") */
-        c.variable = heap_strdup(var->text);
-        c.property = NULL;
+    if (lhs_rc > 0) {
+        /* HAS_LABEL leaf — complete condition, no operator follows */
+        return expr_leaf(c);
     }
 
     return parse_condition_op(p, &c);
@@ -1995,6 +2049,8 @@ static void free_where(cbm_where_clause_t *w) {
             safe_str_free(&w->conditions[i].in_values[j]);
         }
         free(w->conditions[i].in_values);
+        safe_str_free(&w->conditions[i].func);
+        func_args_free(w->conditions[i].args, w->conditions[i].arg_count);
     }
     free(w->conditions);
     safe_str_free(&w->op);
@@ -2384,8 +2440,26 @@ static void binding_set(binding_t *b, const char *var, const cbm_node_t *node) {
     b->var_count++;
 }
 
+static const char *eval_multiarg_func(binding_t *b, const cbm_return_item_t *item, char *buf,
+                                      size_t bufsz);
+
 /* Resolve the actual property value for a condition from a binding */
 static const char *resolve_condition_value(const cbm_condition_t *c, binding_t *b) {
+    /* Multi-arg scalar function LHS: coalesce(f.depth, 0) >= 2 (#874).
+     * Evaluated through the same code path as RETURN projections. The value is
+     * consumed by eval_condition before any other condition is resolved, so a
+     * single thread-local buffer per call is safe. */
+    if (c->func) {
+        static _Thread_local char func_buf[CBM_SZ_512];
+        cbm_return_item_t item = {0};
+        item.variable = c->variable;
+        item.property = c->property;
+        item.func = c->func;
+        item.args = c->args;
+        item.arg_count = c->arg_count;
+        return eval_multiarg_func(b, &item, func_buf, sizeof(func_buf));
+    }
+
     cbm_edge_t *e = binding_get_edge(b, c->variable);
     if (e) {
         return edge_prop(e, c->property);

src/cypher/cypher.h

@@ -181,6 +181,13 @@ typedef struct {
     int rel_count;
 } cbm_pattern_t;
 
+/* One argument to a multi-argument scalar function (coalesce, substring, ...). */
+typedef struct {
+    const char *variable; /* variable reference (NULL if a literal) */
+    const char *property; /* property of the variable (NULL if whole var / literal) */
+    const char *literal;  /* literal string/number text (NULL if a variable ref) */
+} cbm_func_arg_t;
+
 /* WHERE condition */
 typedef struct {
     const char *variable;
@@ -198,6 +205,11 @@ typedef struct {
      * anchor, `value` the edge type (NULL = any), `exists_dir` the direction
      * (0 = outbound, 1 = inbound, 2 = any). */
     int exists_dir;
+    /* Multi-arg scalar function on the LHS, e.g. coalesce(f.depth, 0) >= 2
+     * (#874). NULL func = plain variable/property LHS. */
+    const char *func;
+    cbm_func_arg_t *args;
+    int arg_count;
 } cbm_condition_t;
 
 /* Expression tree for WHERE clause */
@@ -237,13 +249,6 @@ typedef struct {
     const char *else_val; /* NULL if no ELSE */
 } cbm_case_expr_t;
 
-/* One argument to a multi-argument scalar function (coalesce, substring, ...). */
-typedef struct {
-    const char *variable; /* variable reference (NULL if a literal) */
-    const char *property; /* property of the variable (NULL if whole var / literal) */
-    const char *literal;  /* literal string/number text (NULL if a variable ref) */
-} cbm_func_arg_t;
-
 /* RETURN item */
 typedef struct {
     const char *variable;

tests/test_cypher.c

@@ -836,6 +836,123 @@ TEST(cypher_func_multiarg) {
     PASS();
 }
 
+/* issue #874: coalesce() in WHERE — null-safe numeric filter over an optional
+ * JSON property. Exact repro shape from the issue: nodes lacking the property
+ * fall back to the literal default instead of failing to parse. */
+TEST(cypher_issue874_where_coalesce_numeric) {
+    cbm_store_t *s = cbm_store_open_memory();
+    cbm_store_upsert_project(s, "test", "/tmp/test");
+    cbm_node_t n1 = {.project = "test",
+                     .label = "Function",
+                     .name = "DeepLoop",
+                     .qualified_name = "test.DeepLoop",
+                     .file_path = "deep.go",
+                     .properties_json = "{\"transitive_loop_depth\":5}"};
+    cbm_node_t n2 = {.project = "test",
+                     .label = "Function",
+                     .name = "NoMetrics",
+                     .qualified_name = "test.NoMetrics",
+                     .file_path = "flat.go"};
+    cbm_store_upsert_node(s, &n1);
+    cbm_store_upsert_node(s, &n2);
+
+    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); /* only DeepLoop; NoMetrics coalesces to 0 */
+    ASSERT_STR_EQ(r.rows[0][0], "test.DeepLoop");
+    cbm_cypher_result_free(&r);
+    cbm_store_close(s);
+    PASS();
+}
+
+/* issue #874: coalesce() in WHERE with a string fallback and first-arg-wins. */
+TEST(cypher_issue874_where_coalesce_string) {
+    cbm_store_t *s = setup_cypher_store();
+    cbm_cypher_result_t r = {0};
+
+    /* Missing property on every node → fallback literal matches all 4 Functions */
+    int rc = cbm_cypher_execute(
+        s,
+        "MATCH (f:Function) WHERE coalesce(f.missing, \"fallback\") = \"fallback\" "
+        "RETURN f.name",
+        "test", 0, &r);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r.row_count, 4);
+    cbm_cypher_result_free(&r);
+
+    /* Present first arg wins over the fallback */
+    cbm_cypher_result_t r2 = {0};
+    rc = cbm_cypher_execute(
+        s, "MATCH (f:Function) WHERE coalesce(f.name, \"zz\") = \"HandleOrder\" RETURN f.name",
+        "test", 0, &r2);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r2.row_count, 1);
+    ASSERT_STR_EQ(r2.rows[0][0], "HandleOrder");
+    cbm_cypher_result_free(&r2);
+
+    cbm_store_close(s);
+    PASS();
+}
+
+/* issue #874: function LHS composes with NOT and AND like any other condition. */
+TEST(cypher_issue874_where_coalesce_not_and) {
+    cbm_store_t *s = setup_cypher_store();
+    cbm_cypher_result_t r = {0};
+
+    int rc = cbm_cypher_execute(
+        s, "MATCH (f:Function) WHERE NOT coalesce(f.missing, \"x\") = \"x\" RETURN f.name", "test",
+        0, &r);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r.row_count, 0); /* every node coalesces to "x" — NOT filters all */
+    cbm_cypher_result_free(&r);
+
+    cbm_cypher_result_t r2 = {0};
+    rc = cbm_cypher_execute(s,
+                            "MATCH (f:Function) WHERE coalesce(f.missing, \"1\") = \"1\" "
+                            "AND f.name CONTAINS \"Order\" RETURN f.name",
+                            "test", 0, &r2);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r2.row_count, 3); /* HandleOrder, ValidateOrder, SubmitOrder */
+    cbm_cypher_result_free(&r2);
+
+    cbm_store_close(s);
+    PASS();
+}
+
+/* issue #874: the other multi-arg scalar functions work in WHERE too. */
+TEST(cypher_issue874_where_substring) {
+    cbm_store_t *s = setup_cypher_store();
+    cbm_cypher_result_t r = {0};
+    int rc = cbm_cypher_execute(
+        s, "MATCH (f:Function) WHERE substring(f.name, 0, 6) = \"Handle\" RETURN f.name", "test", 0,
+        &r);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r.row_count, 1);
+    ASSERT_STR_EQ(r.rows[0][0], "HandleOrder");
+    cbm_cypher_result_free(&r);
+    cbm_store_close(s);
+    PASS();
+}
+
+/* issue #874: an unrecognised function in WHERE must fail loudly with the
+ * supported set, not the misleading "unexpected operator". */
+TEST(cypher_issue874_where_unsupported_func_error) {
+    cbm_query_t *q = NULL;
+    char *err = NULL;
+    int rc =
+        cbm_cypher_parse("MATCH (f:Function) WHERE foo(f.name) = \"x\" RETURN f.name", &q, &err);
+    ASSERT_EQ(rc, -1);
+    ASSERT_NOT_NULL(err);
+    ASSERT_TRUE(strstr(err, "unsupported function 'foo'") != NULL);
+    free(err);
+    PASS();
+}
+
 TEST(cypher_exists_no_callers) {
     /* NOT EXISTS { (f)<-[:CALLS]-() } → functions with no CALLS caller.
      * HandleOrder has only an incoming DEFINES edge (not CALLS), so it is the
@@ -2882,6 +2999,11 @@ SUITE(cypher) {
     RUN_TEST(cypher_func_tointeger_tofloat);
     RUN_TEST(cypher_func_size_reverse);
     RUN_TEST(cypher_func_multiarg);
+    RUN_TEST(cypher_issue874_where_coalesce_numeric);
+    RUN_TEST(cypher_issue874_where_coalesce_string);
+    RUN_TEST(cypher_issue874_where_coalesce_not_and);
+    RUN_TEST(cypher_issue874_where_substring);
+    RUN_TEST(cypher_issue874_where_unsupported_func_error);
     RUN_TEST(cypher_multi_prop_projection_no_alias);
     RUN_TEST(cypher_exists_no_callers);
     RUN_TEST(cypher_exists_has_outgoing_calls);