Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 96 additions & 45 deletions src/cypher/cypher.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion src/cypher/cypher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 66 additions & 4 deletions tests/test_cypher.c
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading