Skip to content

Commit 63b9997

Browse files
fix(cypher): RETURN * must read the live scope, not the query pattern
RETURN * built its columns from the variables the query pattern named, never from the bindings it was about to project. One line caused two separate wrong answers, and neither one reported an error. After a WITH, the pattern's variables are out of scope — the WITH replaced them with the names it made. The old code still asked for the old names, found none of them, and answered a full result of empty strings. This query used to print twelve columns of nothing: MATCH (f:Function) OPTIONAL MATCH (f)-[:CALLS]->(g) WITH f.name AS caller, g.name AS callee RETURN * It now prints two columns, caller and callee, holding their values. A name the WITH made holds one value rather than a node, so it gets one column, not the four a node variable gets. Separately, collect_pattern_vars appended every pattern's variables with no repeat check. A variable named in two patterns got its four columns twice, which the OPTIONAL MATCH above does with f. Two tests cover both faults and fail against the old code: cypher_return_star_dedups_repeated_pattern_var col_count 12, want 8 cypher_return_star_after_with_names_aliases col_count 8, want 2 Cypher suite: 185 passed, 0 failed. clang-format clean on both files. Reported alongside a second fault this does NOT fix: a variable the WITH dropped is still accepted afterwards and renders empty, because nothing checks a projected name against the live scope. See .agents/research/2026-08-29-cypher-return-star-and-with-scope.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
1 parent c38aa35 commit 63b9997

2 files changed

Lines changed: 100 additions & 4 deletions

File tree

src/cypher/cypher.c

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4193,17 +4193,34 @@ static void execute_with_clause(cbm_query_t *q, binding_t **bindings_ptr, int *b
41934193

41944194
/* Project RETURN * — all bound variable properties */
41954195
/* Collect all variable names from query patterns */
4196+
/* Has this variable already been collected? A query may name the same variable
4197+
* in more than one pattern, and RETURN * must give it one set of columns. */
4198+
static bool star_var_seen(const char **vars, int vc, const char *name) {
4199+
for (int i = 0; i < vc; i++) {
4200+
if (strcmp(vars[i], name) == 0) {
4201+
return true;
4202+
}
4203+
}
4204+
return false;
4205+
}
4206+
4207+
/* Collect the variables a RETURN * projects, in the order the query names them
4208+
* and with no repeats. Without the repeat check, `MATCH (f) OPTIONAL MATCH
4209+
* (f)-[:CALLS]->(g)` names f in two patterns and f gets its four columns
4210+
* twice. */
41964211
static int collect_pattern_vars(cbm_query_t *q, const char **vars, int max_vars) {
41974212
int vc = 0;
41984213
for (int pi = 0; pi < q->pattern_count; pi++) {
41994214
for (int ni = 0; ni < q->patterns[pi].node_count && vc < max_vars; ni++) {
4200-
if (q->patterns[pi].nodes[ni].variable) {
4201-
vars[vc++] = q->patterns[pi].nodes[ni].variable;
4215+
const char *var = q->patterns[pi].nodes[ni].variable;
4216+
if (var && !star_var_seen(vars, vc, var)) {
4217+
vars[vc++] = var;
42024218
}
42034219
}
42044220
for (int ri = 0; ri < q->patterns[pi].rel_count && vc < max_vars; ri++) {
4205-
if (q->patterns[pi].rels[ri].variable) {
4206-
vars[vc++] = q->patterns[pi].rels[ri].variable;
4221+
const char *var = q->patterns[pi].rels[ri].variable;
4222+
if (var && !star_var_seen(vars, vc, var)) {
4223+
vars[vc++] = var;
42074224
}
42084225
}
42094226
}
@@ -4255,8 +4272,41 @@ static void project_star_row(binding_t *b, const char **vars, int vc, const char
42554272
}
42564273
}
42574274

4275+
/* RETURN * after a WITH.
4276+
*
4277+
* The pattern's variables are out of scope by this point — the WITH replaced
4278+
* them with the names it made. Each of those names holds one value, not a
4279+
* node, so each is ONE column rather than the four a node variable gets.
4280+
*
4281+
* Reading the pattern here instead is the fault this function exists to avoid:
4282+
* it named variables the bindings no longer hold, found nothing for every one
4283+
* of them, and answered a full result of empty strings with no error. */
4284+
static void execute_return_star_after_with(cbm_query_t *q, binding_t *bindings, int bind_count,
4285+
int max_rows, result_builder_t *rb) {
4286+
cbm_return_clause_t *wc = q->with_clause;
4287+
char name_bufs[CYP_MAX_VARS][CBM_SZ_128];
4288+
const char *cols[CYP_MAX_VARS];
4289+
int col_n = wc->count < CYP_MAX_VARS ? wc->count : CYP_MAX_VARS;
4290+
for (int i = 0; i < col_n; i++) {
4291+
cols[i] = resolve_item_alias(&wc->items[i], name_bufs[i], sizeof(name_bufs[i]));
4292+
}
4293+
rb_set_columns(rb, cols, col_n);
4294+
for (int bi = 0; bi < bind_count && rb->row_count < max_rows; bi++) {
4295+
const char *vals[CYP_MAX_VARS];
4296+
for (int i = 0; i < col_n; i++) {
4297+
cbm_node_t *vn = binding_get(&bindings[bi], cols[i]);
4298+
vals[i] = vn && vn->name ? vn->name : "";
4299+
}
4300+
rb_add_row(rb, vals);
4301+
}
4302+
}
4303+
42584304
static void execute_return_star(cbm_query_t *q, binding_t *bindings, int bind_count, int max_rows,
42594305
result_builder_t *rb) {
4306+
if (q->with_clause) {
4307+
execute_return_star_after_with(q, bindings, bind_count, max_rows, rb);
4308+
return;
4309+
}
42604310
const char *vars[CBM_SZ_32];
42614311
int vc = collect_pattern_vars(q, vars, CBM_SZ_32);
42624312
build_star_columns(rb, vars, vc);

tests/test_cypher.c

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2982,6 +2982,50 @@ TEST(cypher_exec_return_star) {
29822982
PASS();
29832983
}
29842984

2985+
TEST(cypher_return_star_dedups_repeated_pattern_var) {
2986+
/* RETURN * collected its column variables from every pattern in turn and
2987+
* never deduped, so a variable named in two patterns got its four columns
2988+
* twice. Here f is named in the MATCH and again in the OPTIONAL MATCH, so
2989+
* eight columns is right and twelve is the fault. */
2990+
cbm_store_t *s = setup_cypher_store();
2991+
cbm_cypher_result_t r = {0};
2992+
int rc = cbm_cypher_execute(s, "MATCH (f:Function) OPTIONAL MATCH (f)-[:CALLS]->(g) RETURN *",
2993+
"test", 0, &r);
2994+
ASSERT_EQ(rc, 0);
2995+
ASSERT_EQ(r.col_count, 8);
2996+
ASSERT_STR_EQ(r.columns[0], "f.name");
2997+
ASSERT_STR_EQ(r.columns[4], "g.name");
2998+
cbm_cypher_result_free(&r);
2999+
cbm_store_close(s);
3000+
PASS();
3001+
}
3002+
3003+
TEST(cypher_return_star_after_with_names_aliases) {
3004+
/* RETURN * built its columns from the query pattern, never from the
3005+
* bindings it was about to project. After a WITH the live scope is the
3006+
* aliases the WITH made, so the old code asked for f and g, found neither,
3007+
* and answered every value empty with no error. */
3008+
cbm_store_t *s = setup_cypher_store();
3009+
cbm_cypher_result_t r = {0};
3010+
int rc = cbm_cypher_execute(s,
3011+
"MATCH (f:Function)-[:CALLS]->(g) "
3012+
"WITH f.name AS caller, g.name AS callee RETURN *",
3013+
"test", 0, &r);
3014+
ASSERT_EQ(rc, 0);
3015+
ASSERT_EQ(r.col_count, 2);
3016+
ASSERT_STR_EQ(r.columns[0], "caller");
3017+
ASSERT_STR_EQ(r.columns[1], "callee");
3018+
/* Three CALLS edges in the fixture. */
3019+
ASSERT_EQ(r.row_count, 3);
3020+
for (int i = 0; i < r.row_count; i++) {
3021+
ASSERT_TRUE(r.rows[i][0][0] != '\0');
3022+
ASSERT_TRUE(r.rows[i][1][0] != '\0');
3023+
}
3024+
cbm_cypher_result_free(&r);
3025+
cbm_store_close(s);
3026+
PASS();
3027+
}
3028+
29853029
TEST(cypher_parse_neq) {
29863030
cbm_query_t *q = NULL;
29873031
char *err = NULL;
@@ -4290,6 +4334,8 @@ SUITE(cypher) {
42904334
RUN_TEST(cypher_exec_where_is_null);
42914335
RUN_TEST(cypher_exec_where_is_not_null);
42924336
RUN_TEST(cypher_exec_return_star);
4337+
RUN_TEST(cypher_return_star_dedups_repeated_pattern_var);
4338+
RUN_TEST(cypher_return_star_after_with_names_aliases);
42934339
RUN_TEST(cypher_parse_neq);
42944340
RUN_TEST(cypher_parse_in);
42954341
RUN_TEST(cypher_parse_is_null);

0 commit comments

Comments
 (0)