From 0c0eb95fe1f7b657acca988660d9eb2c0ebdf33a Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sat, 29 Aug 2026 13:52:50 -0400 Subject: [PATCH] fix(cypher): RETURN * must read the live scope, not the query pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: Joshua Richter --- src/cypher/cypher.c | 58 +++++++++++++++++++++++++++++++++++++++++---- tests/test_cypher.c | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 5724f3ac6..957860f3d 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -4156,17 +4156,34 @@ static void execute_with_clause(cbm_query_t *q, binding_t **bindings_ptr, int *b /* Project RETURN * — all bound variable properties */ /* Collect all variable names from query patterns */ +/* Has this variable already been collected? A query may name the same variable + * in more than one pattern, and RETURN * must give it one set of columns. */ +static bool star_var_seen(const char **vars, int vc, const char *name) { + for (int i = 0; i < vc; i++) { + if (strcmp(vars[i], name) == 0) { + return true; + } + } + return false; +} + +/* Collect the variables a RETURN * projects, in the order the query names them + * and with no repeats. Without the repeat check, `MATCH (f) OPTIONAL MATCH + * (f)-[:CALLS]->(g)` names f in two patterns and f gets its four columns + * twice. */ static int collect_pattern_vars(cbm_query_t *q, const char **vars, int max_vars) { int vc = 0; for (int pi = 0; pi < q->pattern_count; pi++) { for (int ni = 0; ni < q->patterns[pi].node_count && vc < max_vars; ni++) { - if (q->patterns[pi].nodes[ni].variable) { - vars[vc++] = q->patterns[pi].nodes[ni].variable; + const char *var = q->patterns[pi].nodes[ni].variable; + if (var && !star_var_seen(vars, vc, var)) { + vars[vc++] = var; } } for (int ri = 0; ri < q->patterns[pi].rel_count && vc < max_vars; ri++) { - if (q->patterns[pi].rels[ri].variable) { - vars[vc++] = q->patterns[pi].rels[ri].variable; + const char *var = q->patterns[pi].rels[ri].variable; + if (var && !star_var_seen(vars, vc, var)) { + vars[vc++] = var; } } } @@ -4218,8 +4235,41 @@ static void project_star_row(binding_t *b, const char **vars, int vc, const char } } +/* RETURN * after a WITH. + * + * The pattern's variables are out of scope by this point — the WITH replaced + * them with the names it made. Each of those names holds one value, not a + * node, so each is ONE column rather than the four a node variable gets. + * + * Reading the pattern here instead is the fault this function exists to avoid: + * it named variables the bindings no longer hold, found nothing for every one + * of them, and answered a full result of empty strings with no error. */ +static void execute_return_star_after_with(cbm_query_t *q, binding_t *bindings, int bind_count, + int max_rows, result_builder_t *rb) { + cbm_return_clause_t *wc = q->with_clause; + char name_bufs[CYP_MAX_VARS][CBM_SZ_128]; + const char *cols[CYP_MAX_VARS]; + int col_n = wc->count < CYP_MAX_VARS ? wc->count : CYP_MAX_VARS; + for (int i = 0; i < col_n; i++) { + cols[i] = resolve_item_alias(&wc->items[i], name_bufs[i], sizeof(name_bufs[i])); + } + rb_set_columns(rb, cols, col_n); + for (int bi = 0; bi < bind_count && rb->row_count < max_rows; bi++) { + const char *vals[CYP_MAX_VARS]; + for (int i = 0; i < col_n; i++) { + cbm_node_t *vn = binding_get(&bindings[bi], cols[i]); + vals[i] = vn && vn->name ? vn->name : ""; + } + rb_add_row(rb, vals); + } +} + static void execute_return_star(cbm_query_t *q, binding_t *bindings, int bind_count, int max_rows, result_builder_t *rb) { + if (q->with_clause) { + execute_return_star_after_with(q, bindings, bind_count, max_rows, rb); + return; + } const char *vars[CBM_SZ_32]; int vc = collect_pattern_vars(q, vars, CBM_SZ_32); build_star_columns(rb, vars, vc); diff --git a/tests/test_cypher.c b/tests/test_cypher.c index c37b2b7d0..d7081c600 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -2859,6 +2859,50 @@ TEST(cypher_exec_return_star) { PASS(); } +TEST(cypher_return_star_dedups_repeated_pattern_var) { + /* RETURN * collected its column variables from every pattern in turn and + * never deduped, so a variable named in two patterns got its four columns + * twice. Here f is named in the MATCH and again in the OPTIONAL MATCH, so + * eight columns is right and twelve is the fault. */ + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, "MATCH (f:Function) OPTIONAL MATCH (f)-[:CALLS]->(g) RETURN *", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.col_count, 8); + ASSERT_STR_EQ(r.columns[0], "f.name"); + ASSERT_STR_EQ(r.columns[4], "g.name"); + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + +TEST(cypher_return_star_after_with_names_aliases) { + /* RETURN * built its columns from the query pattern, never from the + * bindings it was about to project. After a WITH the live scope is the + * aliases the WITH made, so the old code asked for f and g, found neither, + * and answered every value empty with no error. */ + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, + "MATCH (f:Function)-[:CALLS]->(g) " + "WITH f.name AS caller, g.name AS callee RETURN *", + "test", 0, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.col_count, 2); + ASSERT_STR_EQ(r.columns[0], "caller"); + ASSERT_STR_EQ(r.columns[1], "callee"); + /* Three CALLS edges in the fixture. */ + ASSERT_EQ(r.row_count, 3); + for (int i = 0; i < r.row_count; i++) { + ASSERT_TRUE(r.rows[i][0][0] != '\0'); + ASSERT_TRUE(r.rows[i][1][0] != '\0'); + } + cbm_cypher_result_free(&r); + cbm_store_close(s); + PASS(); +} + TEST(cypher_parse_neq) { cbm_query_t *q = NULL; char *err = NULL; @@ -4161,6 +4205,8 @@ SUITE(cypher) { RUN_TEST(cypher_exec_where_is_null); RUN_TEST(cypher_exec_where_is_not_null); RUN_TEST(cypher_exec_return_star); + RUN_TEST(cypher_return_star_dedups_repeated_pattern_var); + RUN_TEST(cypher_return_star_after_with_names_aliases); RUN_TEST(cypher_parse_neq); RUN_TEST(cypher_parse_in); RUN_TEST(cypher_parse_is_null);