From 63b99976a21463697d6e980e32aa42f5913319da Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sat, 29 Aug 2026 13:52:50 -0400 Subject: [PATCH 1/2] 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 17840b04c..82f365b62 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -4193,17 +4193,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; } } } @@ -4255,8 +4272,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 b674da15d..32f9568fe 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -2982,6 +2982,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; @@ -4290,6 +4334,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); From 426e415f5427f2fb2e9c6f918cc7cc9abd9c109b Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Tue, 1 Sep 2026 15:26:21 -0400 Subject: [PATCH 2/2] fix(cypher): refuse a WITH wider than a binding can carry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit execute_return_star_after_with sized its column arrays at CYP_MAX_VARS while the parser allowed up to CBM_SZ_32 items. A WITH naming 17 to 32 aliases parsed cleanly, then answered 16 columns and dropped the rest with no error. The caller could not tell that from a query that genuinely had 16 columns. Widening those arrays to 32 is the wrong fix and would make the result worse. A binding holds exactly CYP_MAX_VARS variables, and with_add_vbinding_var drops any alias past the 16th on the way in. The projection would then report 20 columns of which 4 are always blank — silent wrong data in place of a silent short answer. The 16 was not an arbitrary undersize; it matched what the binding can carry. So bound the WITH where the RETURN is already bounded, in parse_return_or_with, and refuse the query instead of answering it short. The clamp in execute_return_star_after_with stays: it can no longer fire, and it is what keeps that function correct if the bound ever moves. The test asserts both halves — a 20-alias WITH is refused, and a 16-alias WITH still succeeds with 16 columns — so the guard rejects only what the binding genuinely cannot hold. Signed-off-by: Joshua Richter --- src/cypher/cypher.c | 14 ++++++++++++-- tests/test_cypher.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 82f365b62..0654cea70 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -1802,8 +1802,16 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_ /* Projection is materialized per row into fixed-width stack arrays sized at * CBM_SZ_32 columns (execute_return_simple and its siblings). Bound the * parsed item count to that width so an over-wide RETURN is rejected here - * instead of writing past those arrays downstream. */ - if (r->count > CBM_SZ_32) { + * instead of writing past those arrays downstream. + * + * WITH is bounded tighter, by CYP_MAX_VARS. Every item a WITH projects + * becomes one variable of the binding that carries the rest of the query, + * and binding_t holds exactly CYP_MAX_VARS variables. A wider WITH used to + * parse, then lose every alias past the 16th in with_add_vbinding_var and + * answer with silently blank columns. Refuse it here, the same way an + * over-wide RETURN is refused, so the caller sees an error instead of a + * short or empty result. */ + if (r->count > (is_with ? CYP_MAX_VARS : CBM_SZ_32)) { free_return_clause(r); return CBM_NOT_FOUND; } @@ -4286,6 +4294,8 @@ static void execute_return_star_after_with(cbm_query_t *q, binding_t *bindings, cbm_return_clause_t *wc = q->with_clause; char name_bufs[CYP_MAX_VARS][CBM_SZ_128]; const char *cols[CYP_MAX_VARS]; + /* parse_return_or_with refuses a WITH wider than CYP_MAX_VARS, so this + * clamp cannot fire. It stays as the bound this function relies on. */ 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])); diff --git a/tests/test_cypher.c b/tests/test_cypher.c index 32f9568fe..bf4770c02 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -3026,6 +3026,45 @@ TEST(cypher_return_star_after_with_names_aliases) { PASS(); } +TEST(cypher_wide_with_refused_not_truncated) { + /* Every item a WITH projects becomes one variable of the binding that + * carries the rest of the query, and a binding holds CYP_MAX_VARS (16) of + * them. A 20-alias WITH used to parse, drop aliases 17 to 20 inside + * with_add_vbinding_var, and answer RETURN * with 16 columns and no error — + * a short result the caller could not tell from a complete one. It has to + * be refused at parse time instead. */ + char query[1024]; + int off = snprintf(query, sizeof(query), "MATCH (f:Function) WITH "); + for (int i = 0; i < 20; i++) { /* 20 > CYP_MAX_VARS (16) */ + off += snprintf(query + off, sizeof(query) - (size_t)off, "%sf.name AS c%d", i ? ", " : "", + i); + } + snprintf(query + off, sizeof(query) - (size_t)off, " RETURN *"); + + cbm_store_t *s = setup_cypher_store(); + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, query, "test", 0, &r); + ASSERT_TRUE(rc != 0); /* refused, not silently narrowed to 16 columns */ + cbm_cypher_result_free(&r); + + /* The width just under the bound still works, so the guard rejects only + * what the binding genuinely cannot carry. */ + char ok_query[1024]; + off = snprintf(ok_query, sizeof(ok_query), "MATCH (f:Function) WITH "); + for (int i = 0; i < 16; i++) { + off += snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, "%sf.name AS c%d", + i ? ", " : "", i); + } + snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, " RETURN *"); + cbm_cypher_result_t r16 = {0}; + ASSERT_EQ(cbm_cypher_execute(s, ok_query, "test", 0, &r16), 0); + ASSERT_EQ(r16.col_count, 16); + cbm_cypher_result_free(&r16); + + cbm_store_close(s); + PASS(); +} + TEST(cypher_parse_neq) { cbm_query_t *q = NULL; char *err = NULL; @@ -4336,6 +4375,7 @@ SUITE(cypher) { 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_wide_with_refused_not_truncated); RUN_TEST(cypher_parse_neq); RUN_TEST(cypher_parse_in); RUN_TEST(cypher_parse_is_null);