Merge pull request DeusData#991 from harshitaajoshi/fix/cypher-coalesce-where-874

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

Author: DeusData

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,82 +1105,94 @@ static cbm_expr_t *parse_condition_op(parser_t *p, cbm_condition_t *c) {
     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);
-
-    /* EXISTS { pattern } predicate (anchored single-hop existence). */
-    if (check(p, TOK_EXISTS)) {
-        return parse_exists_predicate(p, negated);
+/* 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;
     }
 
-    /* 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);
+    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 NULL;
+        return CBM_NOT_FOUND;
     }
 
-    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;
+            return CBM_NOT_FOUND;
         }
-        c.variable = heap_strdup(var->text);
-        c.op = heap_strdup("HAS_LABEL");
-        c.value = heap_strdup(lbl->text);
-        return expr_leaf(c);
+        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 NULL;
+            return CBM_NOT_FOUND;
         }
-        c.variable = heap_strdup(var->text);
-        c.property = heap_strdup(prop->text);
+        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;
+        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);
+
+    /* EXISTS { pattern } predicate (anchored single-hop existence). */
+    if (check(p, TOK_EXISTS)) {
+        return parse_exists_predicate(p, negated);
+    }
+
+    cbm_condition_t c = {0};
+    c.negated = negated;
+
+    int lhs_rc = parse_condition_lhs(p, &c);
+    if (lhs_rc < 0) {
+        return 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;