From 67fe49d64f5e90dd008c57fb8ff07fe00c72a6ec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:59:33 +0000 Subject: [PATCH 1/5] transform_match: honor $parameters in inline relationship property filters (#96) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MATCH ()-[r:TYPE {prop: $param}]->() silently skipped the parameter and matched every edge of the type — dangerous for SET/DELETE scoped by such a filter. Mirror the node-pattern parameter handling: OR of EXISTS subqueries across the four edge property type tables bound to the named parameter. Literal filters and WHERE-clause parameters were already correct. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QuzbsuTddFm245sZ9egekS --- src/backend/transform/transform_match.c | 40 +++++++++++++++++- tests/test_executor_params.c | 56 +++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/backend/transform/transform_match.c b/src/backend/transform/transform_match.c index 67286ec9..1c057504 100644 --- a/src/backend/transform/transform_match.c +++ b/src/backend/transform/transform_match.c @@ -516,7 +516,45 @@ int transform_match_clause(cypher_transform_context *ctx, cypher_match *match) if (m->pairs) { for (int pi = 0; pi < m->pairs->count; pi++) { cypher_map_pair *pair = (cypher_map_pair*)m->pairs->items[pi]; - if (!pair->key || !pair->value || pair->value->type != AST_NODE_LITERAL) continue; + if (!pair->key || !pair->value) continue; + if (pair->value->type == AST_NODE_PARAMETER) { + /* Parameter in relationship property filter, e.g. + * ()-[r:KNOWS {tag: $t}]->(). Mirror the node-pattern + * parameter handling: OR conditions across each edge + * property type table so string, int, real, and bool + * params all match correctly (GitHub #96 — previously + * parameters here were silently skipped, so the + * filter matched every edge of the type). */ + cypher_parameter *param = (cypher_parameter*)pair->value; + char *esc_key = escape_sql_string(pair->key); + const char *key_sql = esc_key ? esc_key : pair->key; + dynamic_buffer cond; + dbuf_init(&cond); + dbuf_appendf(&cond, + "(" + "EXISTS(SELECT 1 FROM edge_props_text ept " + "JOIN property_keys pk ON ept.key_id = pk.id " + "WHERE ept.edge_id = %s.id AND pk.key = '%s' AND ept.value = :%s) OR " + "EXISTS(SELECT 1 FROM edge_props_int epi " + "JOIN property_keys pk ON epi.key_id = pk.id " + "WHERE epi.edge_id = %s.id AND pk.key = '%s' AND epi.value = :%s) OR " + "EXISTS(SELECT 1 FROM edge_props_real epr " + "JOIN property_keys pk ON epr.key_id = pk.id " + "WHERE epr.edge_id = %s.id AND pk.key = '%s' AND epr.value = :%s) OR " + "EXISTS(SELECT 1 FROM edge_props_bool epb " + "JOIN property_keys pk ON epb.key_id = pk.id " + "WHERE epb.edge_id = %s.id AND pk.key = '%s' AND epb.value = :%s)" + ")", + edge_alias, key_sql, param->name, + edge_alias, key_sql, param->name, + edge_alias, key_sql, param->name, + edge_alias, key_sql, param->name); + free(esc_key); + sql_where(ctx->unified_builder, dbuf_get(&cond)); + dbuf_free(&cond); + continue; + } + if (pair->value->type != AST_NODE_LITERAL) continue; cypher_literal *lit = (cypher_literal*)pair->value; const char *tbl = NULL; char val_buf[256] = ""; diff --git a/tests/test_executor_params.c b/tests/test_executor_params.c index 0acd6526..e583bd7d 100644 --- a/tests/test_executor_params.c +++ b/tests/test_executor_params.c @@ -332,6 +332,58 @@ static void test_null_param(void) if (result) cypher_result_free(result); } +/* + * GitHub #96: inline relationship-pattern property filter with a parameter + * value, e.g. MATCH ()-[r:KNOWS {tag: $t}]->(). Previously the parameter + * was silently skipped and the filter matched every edge of the type. + */ +static void test_rel_inline_param_filter(void) +{ + cypher_result *r; + r = exec("CREATE (:RelParam {name: \"src\"})"); + if (r) cypher_result_free(r); + r = exec("CREATE (:RelParam {name: \"dst\"})"); + if (r) cypher_result_free(r); + r = exec("MATCH (a:RelParam {name: \"src\"}), (b:RelParam {name: \"dst\"}) " + "CREATE (a)-[:RP_KNOWS {w: 1}]->(b)"); + if (r) cypher_result_free(r); + r = exec("MATCH (a:RelParam {name: \"src\"}), (b:RelParam {name: \"dst\"}) " + "CREATE (a)-[:RP_KNOWS {w: 2, tag: \"target\"}]->(b)"); + if (r) cypher_result_free(r); + + /* String parameter: must match only the tagged edge, not all edges. */ + cypher_result *result = exec_params( + "MATCH ()-[e:RP_KNOWS {tag: $t}]->() RETURN e.w AS w", + "{\"t\": \"target\"}" + ); + CU_ASSERT_PTR_NOT_NULL(result); + CU_ASSERT_TRUE(result && result->success); + CU_ASSERT_EQUAL(get_row_count(result), 1); + CU_ASSERT_TRUE(result_contains_value(result, "w", "2")); + if (result) cypher_result_free(result); + + /* Integer parameter. */ + result = exec_params( + "MATCH ()-[e:RP_KNOWS {w: $w}]->() RETURN e.w AS w", + "{\"w\": 1}" + ); + CU_ASSERT_PTR_NOT_NULL(result); + CU_ASSERT_TRUE(result && result->success); + CU_ASSERT_EQUAL(get_row_count(result), 1); + CU_ASSERT_TRUE(result_contains_value(result, "w", "1")); + if (result) cypher_result_free(result); + + /* Non-matching parameter value: no rows. */ + result = exec_params( + "MATCH ()-[e:RP_KNOWS {tag: $t}]->() RETURN e.w AS w", + "{\"t\": \"nope\"}" + ); + CU_ASSERT_PTR_NOT_NULL(result); + CU_ASSERT_TRUE(result && result->success); + CU_ASSERT_EQUAL(get_row_count(result), 0); + if (result) cypher_result_free(result); +} + /* Register all tests */ int register_params_tests(void) { @@ -371,5 +423,9 @@ int register_params_tests(void) if (!CU_add_test(suite, "Null parameter", test_null_param)) return -1; + /* Relationship inline pattern property filter (GitHub #96) */ + if (!CU_add_test(suite, "Relationship inline property filter with parameter", test_rel_inline_param_filter)) + return -1; + return 0; } From b57310e8275d92c72102cce047a50f88bccca819 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:59:40 +0000 Subject: [PATCH 2/5] executor: RETURN of CREATE-introduced variables after MATCH+CREATE (#95) MATCH (x),(y) CREATE (x)-[r:T]->(y) RETURN r raised 'Unknown variable: r' after the CREATE had already committed: the handler re-executed MATCH+RETURN, which has no binding for variables introduced by CREATE. execute_multi_match_create_query can now hand back one variable_map per processed MATCH row (matched + CREATE-introduced bindings). The MATCH+CREATE+RETURN handler detects RETURN items referencing CREATE-only variables and projects them from those row maps (bare var, var.prop, and aggregates, with SKIP/LIMIT), one result row per MATCH row. Queries whose RETURN only references MATCH-bound variables keep the legacy path unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QuzbsuTddFm245sZ9egekS --- src/backend/executor/executor_match.c | 59 ++++++- src/backend/executor/query_dispatch.c | 186 ++++++++++++++++++++++- src/include/executor/executor_internal.h | 9 +- tests/test_query_dispatch.c | 80 ++++++++++ 4 files changed, 325 insertions(+), 9 deletions(-) diff --git a/src/backend/executor/executor_match.c b/src/backend/executor/executor_match.c index 186555bf..30dd8580 100644 --- a/src/backend/executor/executor_match.c +++ b/src/backend/executor/executor_match.c @@ -954,14 +954,25 @@ int bind_match_clause_into_varmap(cypher_executor *executor, cypher_match *match * no-arg wrapper just did this). I-0041 C11. */ int execute_multi_match_create_query(cypher_executor *executor, cypher_query *query, cypher_create *create, cypher_result *result, - variable_map **out_var_map) + variable_map **out_var_map, + variable_map ***out_row_maps, int *out_row_count) { + if (out_row_maps) *out_row_maps = NULL; + if (out_row_count) *out_row_count = 0; if (!executor || !query || !create || !result) { if (out_var_map) *out_var_map = NULL; return -1; } if (out_var_map) *out_var_map = NULL; + /* GitHub #95: when out_row_maps is non-NULL, hand back one variable_map + * per processed MATCH row (matched + CREATE-introduced bindings) so the + * caller can project a RETURN that references CREATE-only variables. + * Caller owns the array and each map. */ + variable_map **row_maps = NULL; + int row_maps_n = 0, row_maps_cap = 0; + const bool collect_rows = (out_row_maps != NULL); + variable_map *var_map = create_variable_map(); if (!var_map) { set_result_error(result, "Failed to create variable map"); @@ -1102,6 +1113,8 @@ int execute_multi_match_create_query(cypher_executor *executor, cypher_query *qu free(row_ids); cypher_transform_free_context(ctx); if (var_map) free_variable_map(var_map); + for (int k = 0; k < row_maps_n; k++) free_variable_map(row_maps[k]); + free(row_maps); return -1; } for (int i = 0; i < vcount2; i++) { @@ -1128,13 +1141,36 @@ int execute_multi_match_create_query(cypher_executor *executor, cypher_query *qu free(row_ids); cypher_transform_free_context(ctx); if (var_map) free_variable_map(var_map); + for (int k = 0; k < row_maps_n; k++) free_variable_map(row_maps[k]); + free(row_maps); return -1; } } } } - if (var_map) free_variable_map(var_map); - var_map = row_vm; + if (collect_rows) { + /* Hand row ownership to the collected array; var_map stays + * NULL and is recreated empty below. */ + if (row_maps_n == row_maps_cap) { + row_maps_cap = row_maps_cap ? row_maps_cap * 2 : 8; + variable_map **grown = realloc(row_maps, row_maps_cap * sizeof(variable_map*)); + if (!grown) { + set_result_error(result, "OOM (row map array)"); + free_variable_map(row_vm); + for (int k = 0; k < rows_n; k++) free(row_ids[k]); + free(row_ids); + cypher_transform_free_context(ctx); + for (int k = 0; k < row_maps_n; k++) free_variable_map(row_maps[k]); + free(row_maps); + return -1; + } + row_maps = grown; + } + row_maps[row_maps_n++] = row_vm; + } else { + if (var_map) free_variable_map(var_map); + var_map = row_vm; + } } for (int k = 0; k < rows_n; k++) free(row_ids[k]); free(row_ids); @@ -1159,11 +1195,24 @@ int execute_multi_match_create_query(cypher_executor *executor, cypher_query *qu } } } + if (collect_rows) { + /* Legacy path binds/creates a single row; expose it as one map. */ + row_maps = malloc(sizeof(variable_map*)); + if (row_maps) { + row_maps[0] = var_map; + row_maps_n = 1; + var_map = NULL; + } + } } + if (out_row_maps) { + *out_row_maps = row_maps; + if (out_row_count) *out_row_count = row_maps_n; + } if (out_var_map) { - *out_var_map = var_map; - } else { + *out_var_map = var_map ? var_map : create_variable_map(); + } else if (var_map) { free_variable_map(var_map); } return 0; diff --git a/src/backend/executor/query_dispatch.c b/src/backend/executor/query_dispatch.c index 1f789f34..0c1b8092 100644 --- a/src/backend/executor/query_dispatch.c +++ b/src/backend/executor/query_dispatch.c @@ -812,7 +812,7 @@ static int handle_match_delete(cypher_executor *executor, cypher_query *query, * scenarios (Match5 [26] setup: rewires (a)-[r]->(b) by creating * (b)-[:LIKES]->(a) and deleting r). */ if (cre) { - if (execute_multi_match_create_query(executor, query, cre, result, NULL) < 0) { + if (execute_multi_match_create_query(executor, query, cre, result, NULL, NULL, NULL) < 0) { return -1; } } @@ -1180,7 +1180,8 @@ static int handle_match_create(cypher_executor *executor, cypher_query *query, * legacy execute_match_create_query path. */ variable_map *mc_vars = NULL; int rc = execute_multi_match_create_query(executor, query, create, result, - set ? &mc_vars : NULL); + set ? &mc_vars : NULL, + NULL, NULL); if (rc < 0) { if (mc_vars) free_variable_map(mc_vars); return rc; @@ -1197,6 +1198,53 @@ static int handle_match_create(cypher_executor *executor, cypher_query *query, return rc; } +/* GitHub #95 helpers: detect RETURN items that reference a variable bound + * only by a CREATE pattern (not by any MATCH), e.g. + * MATCH (x), (y) CREATE (x)-[r:T]->(y) RETURN r + * The legacy path re-executes MATCH+RETURN, which has no binding for `r` + * and errored AFTER the CREATE had already committed. */ +static bool pattern_list_binds_var(ast_list *pattern, const char *name) +{ + if (!pattern || !name) return false; + for (int i = 0; i < pattern->count; i++) { + ast_node *p = pattern->items[i]; + if (!p || p->type != AST_NODE_PATH) continue; + cypher_path *path = (cypher_path*)p; + if (path->var_name && strcmp(path->var_name, name) == 0) return true; + if (!path->elements) continue; + for (int j = 0; j < path->elements->count; j++) { + ast_node *el = path->elements->items[j]; + if (!el) continue; + if (el->type == AST_NODE_NODE_PATTERN) { + cypher_node_pattern *np = (cypher_node_pattern*)el; + if (np->variable && strcmp(np->variable, name) == 0) return true; + } else if (el->type == AST_NODE_REL_PATTERN) { + cypher_rel_pattern *rp = (cypher_rel_pattern*)el; + if (rp->variable && strcmp(rp->variable, name) == 0) return true; + } + } + } + return false; +} + +/* Base variable of a var-map-projectable RETURN expression: a bare + * identifier (`r`) or a simple property access (`r.prop`). NULL for + * anything else. */ +static const char *projectable_base_var(ast_node *expr) +{ + if (!expr) return NULL; + if (expr->type == AST_NODE_IDENTIFIER) { + return ((cypher_identifier*)expr)->name; + } + if (expr->type == AST_NODE_PROPERTY) { + cypher_property *prop = (cypher_property*)expr; + if (prop->expr && prop->expr->type == AST_NODE_IDENTIFIER) { + return ((cypher_identifier*)prop->expr)->name; + } + } + return NULL; +} + static int handle_match_create_return(cypher_executor *executor, cypher_query *query, cypher_result *result, clause_flags flags) { @@ -1206,6 +1254,140 @@ static int handle_match_create_return(cypher_executor *executor, cypher_query *q cypher_return *ret = find_return_clause(query); CYPHER_DEBUG("Executing MATCH+CREATE+RETURN via pattern dispatch"); + + /* GitHub #95: if the RETURN references CREATE-only variables, project it + * from the per-row variable maps instead of re-running MATCH+RETURN + * (which cannot see them and would error after the write committed). + * Only taken when every RETURN item is a var-map-projectable shape + * (bare var, var.prop, or an aggregate over those), so all other + * queries keep the legacy path unchanged. */ + bool any_create_only = false; + bool all_projectable = true; + if (ret && ret->items && !ret->return_all && !ret->order_by && query->clauses) { + for (int i = 0; i < ret->items->count; i++) { + cypher_return_item *item = (cypher_return_item*)ret->items->items[i]; + ast_node *expr = item ? item->expr : NULL; + if (aggregating_call_name(expr)) { + cypher_function_call *fc = (cypher_function_call*)expr; + expr = (fc->args && fc->args->count > 0) ? fc->args->items[0] : NULL; + if (!expr) continue; /* count(*) — no variable reference */ + } + const char *base = projectable_base_var(expr); + if (!base) { all_projectable = false; break; } + bool in_match = false; + for (int ci = 0; ci < query->clauses->count; ci++) { + ast_node *c = query->clauses->items[ci]; + if (c && c->type == AST_NODE_MATCH && + pattern_list_binds_var(((cypher_match*)c)->pattern, base)) { + in_match = true; + break; + } + } + if (!in_match) { + bool in_create = false; + for (int ci = 0; ci < query->clauses->count; ci++) { + ast_node *c = query->clauses->items[ci]; + if (c && c->type == AST_NODE_CREATE && + pattern_list_binds_var(((cypher_create*)c)->pattern, base)) { + in_create = true; + break; + } + } + if (in_create) { + any_create_only = true; + } else { + all_projectable = false; + break; + } + } + } + } else { + all_projectable = false; + } + + if (any_create_only && all_projectable) { + variable_map **maps = NULL; + int n_maps = 0; + if (execute_multi_match_create_query(executor, query, create, result, + NULL, &maps, &n_maps) < 0) { + return -1; + } + + int col_count = ret->items->count; + set_return_column_names(ret, result); + + /* SKIP/LIMIT */ + int64_t limit_val = -1, skip_val = 0; + if (ret->limit && ret->limit->type == AST_NODE_LITERAL) { + cypher_literal *l = (cypher_literal*)ret->limit; + if (l->literal_type == LITERAL_INTEGER) limit_val = l->value.integer; + } + if (ret->skip && ret->skip->type == AST_NODE_LITERAL) { + cypher_literal *l = (cypher_literal*)ret->skip; + if (l->literal_type == LITERAL_INTEGER) skip_val = l->value.integer; + } + int start = 0; + if (skip_val > 0) start = (skip_val >= n_maps) ? n_maps : (int)skip_val; + int end = n_maps; + if (limit_val == 0) end = start; + else if (limit_val > 0 && start + (int)limit_val < end) end = start + (int)limit_val; + int produced = end - start; + if (produced < 0) produced = 0; + + if (return_has_aggregation(ret)) { + /* Single aggregated row across the (post-skip/limit) maps. */ + result->row_count = 1; + result->data = malloc(sizeof(char**)); + result->data_types = malloc(sizeof(int*)); + result->data[0] = malloc(col_count * sizeof(char*)); + result->data_types[0] = calloc(col_count, sizeof(int)); + for (int i = 0; i < col_count; i++) { + cypher_return_item *it = (cypher_return_item*)ret->items->items[i]; + if (aggregating_call_name(it->expr)) { + project_aggregate_cell(executor, it, maps + start, produced, result, i); + } else if (produced > 0) { + char **save_data = result->data[0]; + int *save_types = result->data_types[0]; + char ***save_data_all = result->data; + int **save_types_all = result->data_types; + char **tmp = malloc(col_count * sizeof(char*)); + int *tmp_t = calloc(col_count, sizeof(int)); + result->data = &tmp; + result->data_types = &tmp_t; + project_return_row_from_var_map(executor, ret, maps[start], result, 0); + result->data = save_data_all; + result->data_types = save_types_all; + save_data[i] = tmp[i]; + save_types[i] = tmp_t[i]; + for (int k = 0; k < col_count; k++) if (k != i && tmp[k]) free(tmp[k]); + free(tmp); + free(tmp_t); + } else { + result->data[0][i] = NULL; + } + } + } else { + result->row_count = produced; + if (produced == 0) { + result->data = NULL; + result->data_types = NULL; + } else { + result->data = malloc(produced * sizeof(char**)); + result->data_types = malloc(produced * sizeof(int*)); + for (int r = 0; r < produced; r++) { + result->data[r] = malloc(col_count * sizeof(char*)); + result->data_types[r] = calloc(col_count, sizeof(int)); + project_return_row_from_var_map(executor, ret, maps[start + r], result, r); + } + } + } + + for (int j = 0; j < n_maps; j++) free_variable_map(maps[j]); + free(maps); + result->success = true; + return 0; + } + int rc = execute_match_create_return_query(executor, match, create, ret, result); if (rc >= 0) { result->success = true; diff --git a/src/include/executor/executor_internal.h b/src/include/executor/executor_internal.h index ce240add..1cc48b87 100644 --- a/src/include/executor/executor_internal.h +++ b/src/include/executor/executor_internal.h @@ -126,10 +126,15 @@ int execute_set_items(cypher_executor *executor, ast_list *items, variable_map * int execute_match_return_query(cypher_executor *executor, cypher_match *match, cypher_return *return_clause, cypher_result *result); int execute_match_create_query(cypher_executor *executor, cypher_match *match, cypher_create *create, cypher_result *result); /* Canonical multi-MATCH + CREATE entry point (I-0041 C11). Pass NULL - * for out_var_map if the caller doesn't need the accumulated bindings. */ + * for out_var_map if the caller doesn't need the accumulated bindings. + * Pass non-NULL out_row_maps/out_row_count to receive one variable_map per + * processed MATCH row (matched + CREATE-introduced bindings) for RETURN + * projection of CREATE-only variables (GitHub #95); caller owns the array + * and each map. */ int execute_multi_match_create_query(cypher_executor *executor, cypher_query *query, cypher_create *create, cypher_result *result, - variable_map **out_var_map); + variable_map **out_var_map, + variable_map ***out_row_maps, int *out_row_count); int bind_match_clause_into_varmap(cypher_executor *executor, cypher_match *match, variable_map *var_map, cypher_result *result); int execute_match_merge_query_with_varmap(cypher_executor *executor, cypher_match *match, cypher_merge *merge, diff --git a/tests/test_query_dispatch.c b/tests/test_query_dispatch.c index 2822bc61..a02d1b75 100644 --- a/tests/test_query_dispatch.c +++ b/tests/test_query_dispatch.c @@ -404,6 +404,85 @@ static void test_dispatch_match_return(void) } } +/* + * GitHub #95: RETURN of a relationship variable created between + * MATCH-bound endpoints. Previously raised "Unknown variable: r" after + * the CREATE had already committed. + */ +static void test_dispatch_match_create_return_created_var(void) +{ + cypher_executor *executor = cypher_executor_create(dispatch_test_db); + CU_ASSERT_PTR_NOT_NULL(executor); + if (executor) { + cypher_result *r1 = cypher_executor_execute(executor, "CREATE (n:DispMCR {name: \"h0\"})"); + if (r1) cypher_result_free(r1); + cypher_result *r2 = cypher_executor_execute(executor, "CREATE (n:DispMCR {name: \"h1\"})"); + if (r2) cypher_result_free(r2); + + /* RETURN r.prop on the created relationship */ + cypher_result *result = cypher_executor_execute(executor, + "MATCH (x:DispMCR {name: \"h0\"}), (y:DispMCR {name: \"h1\"}) " + "CREATE (x)-[r:DISP_LINKS {seq: 7}]->(y) RETURN r.seq AS s"); + CU_ASSERT_PTR_NOT_NULL(result); + if (result) { + CU_ASSERT_TRUE(result->success); + if (result->success) { + CU_ASSERT_EQUAL(result->row_count, 1); + if (result->row_count == 1 && result->data[0][0]) { + CU_ASSERT_STRING_EQUAL(result->data[0][0], "7"); + } + } + cypher_result_free(result); + } + + /* Bare RETURN r on the created relationship */ + result = cypher_executor_execute(executor, + "MATCH (x:DispMCR {name: \"h0\"}), (y:DispMCR {name: \"h1\"}) " + "CREATE (x)-[r:DISP_LINKS2 {seq: 8}]->(y) RETURN r"); + CU_ASSERT_PTR_NOT_NULL(result); + if (result) { + CU_ASSERT_TRUE(result->success); + if (result->success) { + CU_ASSERT_EQUAL(result->row_count, 1); + if (result->row_count == 1 && result->data[0][0]) { + CU_ASSERT_PTR_NOT_NULL(strstr(result->data[0][0], "DISP_LINKS2")); + } + } + cypher_result_free(result); + } + + /* Two separate MATCH clauses feeding the CREATE */ + result = cypher_executor_execute(executor, + "MATCH (x:DispMCR {name: \"h0\"}) MATCH (y:DispMCR {name: \"h1\"}) " + "CREATE (x)-[r:DISP_LINKS3 {seq: 9}]->(y) RETURN r.seq AS s"); + CU_ASSERT_PTR_NOT_NULL(result); + if (result) { + CU_ASSERT_TRUE(result->success); + if (result->success) { + CU_ASSERT_EQUAL(result->row_count, 1); + if (result->row_count == 1 && result->data[0][0]) { + CU_ASSERT_STRING_EQUAL(result->data[0][0], "9"); + } + } + cypher_result_free(result); + } + + /* Exactly one edge per CREATE (no duplicates from the RETURN pass) */ + result = cypher_executor_execute(executor, + "MATCH ()-[e:DISP_LINKS]->() RETURN count(e) AS c"); + CU_ASSERT_PTR_NOT_NULL(result); + if (result) { + CU_ASSERT_TRUE(result->success); + if (result->success && result->row_count == 1 && result->data[0][0]) { + CU_ASSERT_STRING_EQUAL(result->data[0][0], "1"); + } + cypher_result_free(result); + } + + cypher_executor_free(executor); + } +} + static void test_dispatch_match_set(void) { cypher_executor *executor = cypher_executor_create(dispatch_test_db); @@ -530,6 +609,7 @@ int init_query_dispatch_suite(void) if (!CU_add_test(dispatch_suite, "dispatch: CREATE", test_dispatch_create)) return -1; if (!CU_add_test(dispatch_suite, "dispatch: MATCH+RETURN", test_dispatch_match_return)) return -1; + if (!CU_add_test(dispatch_suite, "dispatch: MATCH+CREATE+RETURN created rel var", test_dispatch_match_create_return_created_var)) return -1; if (!CU_add_test(dispatch_suite, "dispatch: MATCH+SET", test_dispatch_match_set)) return -1; if (!CU_add_test(dispatch_suite, "dispatch: MATCH+DELETE", test_dispatch_match_delete)) return -1; if (!CU_add_test(dispatch_suite, "dispatch: MERGE", test_dispatch_merge)) return -1; From c6d2ad67a1648dac6dd2c45b315dcfb2c69eb647 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:59:48 +0000 Subject: [PATCH 3/5] upsert_edge: caller-assigned edge ids for parallel edges (#97) MERGE previously ignored parameter-valued inline properties: the match phase skipped them (matching any node of the label / any edge on the triple) and node creation dropped them. Fix find_node_by_pattern, find_edge_by_pattern, and the node-create property phase to resolve $parameters, so MERGE (a)-[r:T {id: $eid}]->(b) matches/creates by that property like a literal would. On top of that, expose caller-assigned edge identities in the bindings: - Python: upsert_edge(..., edge_id=None). With edge_id, the edge is merged on that id (stored as an 'id' relationship property) instead of the (source, target, rel_type) triple, so parallel edges on the same triple are individually addressable and upsertable in place. Default behavior is unchanged. - Rust: new upsert_edge_with_id(source, target, props, rel_type, edge_id) with the same semantics; upsert_edge is untouched. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QuzbsuTddFm245sZ9egekS --- bindings/python/src/graphqlite/graph/edges.py | 43 +++- bindings/python/tests/test_graph.py | 51 +++++ bindings/rust/src/graph/edges.rs | 62 ++++++ bindings/rust/tests/integration.rs | 38 ++++ src/backend/executor/executor_merge.c | 200 +++++++++++++++++- tests/test_executor_merge.c | 79 ++++++- 6 files changed, 460 insertions(+), 13 deletions(-) diff --git a/bindings/python/src/graphqlite/graph/edges.py b/bindings/python/src/graphqlite/graph/edges.py index 353c12d5..5758ed3c 100644 --- a/bindings/python/src/graphqlite/graph/edges.py +++ b/bindings/python/src/graphqlite/graph/edges.py @@ -60,14 +60,23 @@ def upsert_edge( source_id: str, target_id: str, edge_data: dict[str, Any], - rel_type: str = "RELATED" + rel_type: str = "RELATED", + edge_id: Optional[str] = None, ) -> None: """ Create or update an edge between two nodes. - If an edge of the same type already exists, its properties are updated - (merge semantics -- existing properties not in edge_data are preserved). - If no edge of that type exists, a new one is created. + Without edge_id: if an edge of the same type already exists, its + properties are updated (merge semantics -- existing properties not in + edge_data are preserved). If no edge of that type exists, a new one is + created. + + With edge_id: the edge is matched/merged on that caller-assigned id + (stored as an ``id`` property on the relationship) instead of on the + (source, target, rel_type) triple. Repeated calls with the same + edge_id update that edge in place; different edge_ids create distinct + parallel edges between the same two nodes with the same type. + Both source and target nodes must exist. Args: @@ -75,17 +84,29 @@ def upsert_edge( target_id: Target node id edge_data: Dictionary of edge properties rel_type: Relationship type label + edge_id: Optional caller-assigned edge identifier """ safe_rel_type = sanitize_rel_type(rel_type) - self._conn.cypher( - f"MATCH (a {{id: $src}}), (b {{id: $tgt}}) " - f"MERGE (a)-[r:{safe_rel_type}]->(b)", - params={"src": source_id, "tgt": target_id}, - ) + if edge_id is None: + self._conn.cypher( + f"MATCH (a {{id: $src}}), (b {{id: $tgt}}) " + f"MERGE (a)-[r:{safe_rel_type}]->(b)", + params={"src": source_id, "tgt": target_id}, + ) + rel_match = f"[r:{safe_rel_type}]" + base_params = {"src": source_id, "tgt": target_id} + else: + self._conn.cypher( + f"MATCH (a {{id: $src}}), (b {{id: $tgt}}) " + f"MERGE (a)-[r:{safe_rel_type} {{id: $eid}}]->(b)", + params={"src": source_id, "tgt": target_id, "eid": edge_id}, + ) + rel_match = f"[r:{safe_rel_type} {{id: $eid}}]" + base_params = {"src": source_id, "tgt": target_id, "eid": edge_id} if edge_data: - params = {"src": source_id, "tgt": target_id} + params = dict(base_params) set_parts = [] for i, (k, v) in enumerate(edge_data.items()): param_name = f"v{i}" @@ -93,7 +114,7 @@ def upsert_edge( params[param_name] = v set_str = ", ".join(set_parts) self._conn.cypher( - f"MATCH (a {{id: $src}})-[r:{safe_rel_type}]->" + f"MATCH (a {{id: $src}})-{rel_match}->" f"(b {{id: $tgt}}) SET {set_str}", params=params, ) diff --git a/bindings/python/tests/test_graph.py b/bindings/python/tests/test_graph.py index 5b800613..1f739820 100644 --- a/bindings/python/tests/test_graph.py +++ b/bindings/python/tests/test_graph.py @@ -252,6 +252,57 @@ def test_upsert_edge_update_empty_props(g): assert edge["properties"]["weight"] == 1 +def test_upsert_edge_with_edge_id_parallel_edges(g): + """GitHub #97: a caller-assigned edge_id addresses parallel edges on the + same (source, target, type) triple.""" + g.upsert_node("a", {"name": "A"}) + g.upsert_node("b", {"name": "B"}) + + # Two distinct edge_ids on the same triple -> two parallel edges + g.upsert_edge("a", "b", {"seq": 1}, rel_type="KNOWS", edge_id="k1") + g.upsert_edge("a", "b", {"seq": 2}, rel_type="KNOWS", edge_id="k2") + + rows = g.query( + "MATCH (a {id: 'a'})-[r:KNOWS]->(b {id: 'b'}) " + "RETURN r.id AS eid, r.seq AS seq" + ) + assert len(rows) == 2 + by_id = {row["eid"]: row["seq"] for row in rows} + assert by_id == {"k1": 1, "k2": 2} + + +def test_upsert_edge_with_edge_id_upserts_in_place(g): + """GitHub #97: repeating an edge_id updates that edge, not a new one.""" + g.upsert_node("a", {"name": "A"}) + g.upsert_node("b", {"name": "B"}) + + g.upsert_edge("a", "b", {"seq": 1}, rel_type="KNOWS", edge_id="k1") + g.upsert_edge("a", "b", {"seq": 10, "note": "updated"}, rel_type="KNOWS", edge_id="k1") + + rows = g.query( + "MATCH (a {id: 'a'})-[r:KNOWS]->(b {id: 'b'}) " + "RETURN r.seq AS seq, r.note AS note" + ) + assert len(rows) == 1 + assert rows[0]["seq"] == 10 + assert rows[0]["note"] == "updated" + + +def test_upsert_edge_without_edge_id_keeps_triple_semantics(g): + """Without edge_id, repeated upserts still merge on the triple.""" + g.upsert_node("a", {"name": "A"}) + g.upsert_node("b", {"name": "B"}) + + g.upsert_edge("a", "b", {"w": 1}, rel_type="KNOWS") + g.upsert_edge("a", "b", {"w": 2}, rel_type="KNOWS") + + rows = g.query( + "MATCH (a {id: 'a'})-[r:KNOWS]->(b {id: 'b'}) RETURN r.w AS w" + ) + assert len(rows) == 1 + assert rows[0]["w"] == 2 + + def test_get_edge_by_type(g): """get_edge should be able to retrieve a specific edge type.""" g.upsert_node("a", {"name": "A"}) diff --git a/bindings/rust/src/graph/edges.rs b/bindings/rust/src/graph/edges.rs index bc97fd4d..9f58ed01 100644 --- a/bindings/rust/src/graph/edges.rs +++ b/bindings/rust/src/graph/edges.rs @@ -108,6 +108,68 @@ impl Graph { Ok(()) } + /// Create or update an edge identified by a caller-assigned edge id. + /// + /// The edge is matched/merged on `edge_id` (stored as an `id` property on + /// the relationship) instead of on the (source, target, rel_type) triple, + /// so multiple parallel edges can exist between the same two nodes with + /// the same relationship type. Repeated calls with the same `edge_id` + /// update that edge's properties in place; different `edge_id`s create + /// distinct edges. + /// + /// Both source and target nodes must exist. + pub fn upsert_edge_with_id( + &self, + source_id: &str, + target_id: &str, + props: I, + rel_type: &str, + edge_id: &str, + ) -> Result<()> + where + I: IntoIterator, + K: AsRef, + V: Into, + { + let safe_rel_type = sanitize_rel_type(rel_type); + + let props: Vec<(String, PropertyValue)> = props + .into_iter() + .map(|(k, v)| (k.as_ref().to_string(), v.into())) + .collect(); + + let merge_query = format!( + "MATCH (a {{id: $src}}), (b {{id: $tgt}}) MERGE (a)-[r:{} {{id: $eid}}]->(b)", + safe_rel_type + ); + self.connection() + .cypher_builder(&merge_query) + .param("src", source_id) + .param("tgt", target_id) + .param("eid", edge_id) + .run()?; + + if !props.is_empty() { + let set_parts: Vec = props + .iter() + .map(|(k, v)| format!("r.{} = {}", k, v.to_cypher())) + .collect(); + let set_str = set_parts.join(", "); + let set_query = format!( + "MATCH (a {{id: $src}})-[r:{} {{id: $eid}}]->(b {{id: $tgt}}) SET {}", + safe_rel_type, set_str + ); + self.connection() + .cypher_builder(&set_query) + .param("src", source_id) + .param("tgt", target_id) + .param("eid", edge_id) + .run()?; + } + + Ok(()) + } + /// Delete the directed edge between two nodes. pub fn delete_edge( &self, diff --git a/bindings/rust/tests/integration.rs b/bindings/rust/tests/integration.rs index 2b6aaef9..cb9b337b 100644 --- a/bindings/rust/tests/integration.rs +++ b/bindings/rust/tests/integration.rs @@ -284,6 +284,44 @@ fn test_graph_upsert_edge() { assert!(!g.has_edge("b", "a", None).unwrap()); // Directed edge } +#[test] +fn test_graph_upsert_edge_with_id() { + // GitHub #97: caller-assigned edge ids address parallel edges on the + // same (source, target, type) triple. + let g = test_graph(); + + g.upsert_node("a", [("name", "A")], "Node").unwrap(); + g.upsert_node("b", [("name", "B")], "Node").unwrap(); + + // Two distinct edge_ids on the same triple -> two parallel edges + g.upsert_edge_with_id("a", "b", [("seq", "1")], "KNOWS", "k1") + .unwrap(); + g.upsert_edge_with_id("a", "b", [("seq", "2")], "KNOWS", "k2") + .unwrap(); + + let result = g + .connection() + .cypher("MATCH (a {id: 'a'})-[r:KNOWS]->(b {id: 'b'}) RETURN r.id AS eid") + .unwrap(); + assert_eq!(result.len(), 2); + + // Repeating an edge_id updates that edge in place + g.upsert_edge_with_id("a", "b", [("seq", "10")], "KNOWS", "k1") + .unwrap(); + let result = g + .connection() + .cypher("MATCH (a {id: 'a'})-[r:KNOWS]->(b {id: 'b'}) RETURN r.id AS eid") + .unwrap(); + assert_eq!(result.len(), 2); + let result = g + .connection() + .cypher("MATCH ()-[r:KNOWS {id: 'k1'}]->() RETURN r.seq AS seq") + .unwrap(); + assert_eq!(result.len(), 1); + let seq: i64 = result[0].get("seq").unwrap_or(0); + assert_eq!(seq, 10); +} + #[test] fn test_graph_stats() { let g = test_graph(); diff --git a/src/backend/executor/executor_merge.c b/src/backend/executor/executor_merge.c index 64f43bb0..518331ff 100644 --- a/src/backend/executor/executor_merge.c +++ b/src/backend/executor/executor_merge.c @@ -78,6 +78,53 @@ typedef struct { #define MAX_BINDINGS 64 +/* Set a node property from a $parameter value during the MERGE create + * phase. Previously parameter-valued inline properties (e.g. + * MERGE (n:L {id: $id})) were silently dropped at creation, leaving the + * new node without the property. */ +static void set_node_prop_from_param(cypher_executor *executor, int node_id, + const char *key, cypher_parameter *param, + cypher_result *result) +{ + if (!executor->params_json) return; + property_type ptype; + property_value pv; + property_value_init(&pv); + if (get_param_value(executor->params_json, param->name, &ptype, &pv) != 0) { + property_value_free(&pv); + return; + } + const void *prop_value = NULL; + int64_t ibuf; + double rbuf; + int bbuf; + switch (ptype) { + case PROP_TYPE_TEXT: + case PROP_TYPE_JSON: + prop_value = pv.as_str; + break; + case PROP_TYPE_INTEGER: + ibuf = pv.as_int; + prop_value = &ibuf; + break; + case PROP_TYPE_REAL: + rbuf = pv.as_real; + prop_value = &rbuf; + break; + case PROP_TYPE_BOOLEAN: + bbuf = pv.as_bool; + prop_value = &bbuf; + break; + default: + break; + } + if (prop_value && + cypher_schema_set_node_property(executor->schema_mgr, node_id, key, ptype, prop_value) == 0) { + result->properties_set++; + } + property_value_free(&pv); +} + /* Helper to bind all parameters to a prepared statement */ static int bind_all_params(sqlite3_stmt *stmt, param_binding *bindings, int count) { @@ -113,6 +160,10 @@ int find_node_by_pattern(cypher_executor *executor, cypher_node_pattern *node_pa int offset = 0; param_binding bindings[MAX_BINDINGS]; int bind_count = 0; + /* Strings resolved from $parameters; must stay alive until after + * sqlite3_step (BIND_TEXT uses SQLITE_STATIC). */ + char *owned_strs[MAX_BINDINGS]; + int owned_count = 0; offset += snprintf(sql + offset, sizeof(sql) - offset, "SELECT n.id FROM nodes n"); @@ -159,6 +210,67 @@ int find_node_by_pattern(cypher_executor *executor, cypher_node_pattern *node_pa } else { free(resolved); } + } else if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER && + executor->params_json) { + /* Parameter-valued inline property, e.g. + * MERGE (n:L {id: $id}). Previously skipped, so the + * match phase ignored the filter entirely (same family + * as GitHub #96) and MERGE could match an unrelated + * node of the label. Resolve the parameter and add the + * same join a literal value would get. */ + cypher_parameter *param = (cypher_parameter*)pair->value; + property_type ptype; + property_value pv; + property_value_init(&pv); + if (get_param_value(executor->params_json, param->name, &ptype, &pv) != 0) { + property_value_free(&pv); + continue; + } + if (bind_count + 2 > MAX_BINDINGS || owned_count >= MAX_BINDINGS) { + property_value_free(&pv); + break; + } + const char *prop_table = NULL; + param_binding val_bind; + switch (ptype) { + case PROP_TYPE_TEXT: + prop_table = "node_props_text"; + val_bind.type = BIND_TEXT; + val_bind.value.text = pv.as_str; + owned_strs[owned_count++] = pv.as_str; + pv.as_str = NULL; /* ownership moved */ + break; + case PROP_TYPE_INTEGER: + prop_table = "node_props_int"; + val_bind.type = BIND_INT; + val_bind.value.integer = (int)pv.as_int; + break; + case PROP_TYPE_REAL: + prop_table = "node_props_real"; + val_bind.type = BIND_DOUBLE; + val_bind.value.real = pv.as_real; + break; + case PROP_TYPE_BOOLEAN: + prop_table = "node_props_bool"; + val_bind.type = BIND_INT; + val_bind.value.integer = pv.as_bool ? 1 : 0; + break; + default: + break; + } + property_value_free(&pv); + if (!prop_table) continue; + offset += snprintf(sql + offset, sizeof(sql) - offset, + " JOIN %s np%d ON n.id = np%d.node_id" + " JOIN property_keys pk%d ON np%d.key_id = pk%d.id AND pk%d.key = ?" + " AND np%d.value = ?", + prop_table, i, i, i, i, i, i, i); + bindings[bind_count].type = BIND_TEXT; + bindings[bind_count].value.text = pair->key; + bind_count++; + bindings[bind_count] = val_bind; + bind_count++; } else if (pair->key && pair->value && pair->value->type == AST_NODE_LITERAL) { cypher_literal *lit = (cypher_literal*)pair->value; @@ -241,12 +353,14 @@ int find_node_by_pattern(cypher_executor *executor, cypher_node_pattern *node_pa int rc = sqlite3_prepare_v2(executor->db, sql, -1, &stmt, NULL); if (rc != SQLITE_OK) { CYPHER_DEBUG("MERGE find query prepare failed: %s", sqlite3_errmsg(executor->db)); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return -1; } if (bind_all_params(stmt, bindings, bind_count) != 0) { CYPHER_DEBUG("MERGE find query bind failed"); sqlite3_finalize(stmt); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return -1; } @@ -257,6 +371,7 @@ int find_node_by_pattern(cypher_executor *executor, cypher_node_pattern *node_pa } sqlite3_finalize(stmt); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return node_id; } @@ -273,6 +388,10 @@ int find_edge_by_pattern(cypher_executor *executor, int source_id, int target_id int offset = 0; param_binding bindings[MAX_BINDINGS]; int bind_count = 0; + /* Strings resolved from $parameters; must stay alive until after + * sqlite3_step (BIND_TEXT uses SQLITE_STATIC). */ + char *owned_strs[MAX_BINDINGS]; + int owned_count = 0; /* source_id and target_id are integers from our own code, safe to interpolate. * For an undirected pattern (a)-[r]-(b) the existing edge may run either @@ -303,7 +422,67 @@ int find_edge_by_pattern(cypher_executor *executor, int source_id, int target_id if (map->pairs) { for (int i = 0; i < map->pairs->count; i++) { cypher_map_pair *pair = (cypher_map_pair*)map->pairs->items[i]; - if (pair->key && pair->value && pair->value->type == AST_NODE_LITERAL) { + if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER && + executor->params_json) { + /* Parameter-valued inline property, e.g. + * MERGE (a)-[r:T {id: $eid}]->(b). Previously skipped, + * so MERGE matched any existing edge on the triple and + * never created a second one (same family as GitHub + * #96; needed for #97-style caller-assigned edge ids). */ + cypher_parameter *param = (cypher_parameter*)pair->value; + property_type ptype; + property_value pv; + property_value_init(&pv); + if (get_param_value(executor->params_json, param->name, &ptype, &pv) != 0) { + property_value_free(&pv); + continue; + } + if (bind_count + 2 > MAX_BINDINGS || owned_count >= MAX_BINDINGS) { + property_value_free(&pv); + break; + } + const char *prop_table = NULL; + param_binding val_bind; + switch (ptype) { + case PROP_TYPE_TEXT: + prop_table = "edge_props_text"; + val_bind.type = BIND_TEXT; + val_bind.value.text = pv.as_str; + owned_strs[owned_count++] = pv.as_str; + pv.as_str = NULL; /* ownership moved */ + break; + case PROP_TYPE_INTEGER: + prop_table = "edge_props_int"; + val_bind.type = BIND_INT; + val_bind.value.integer = (int)pv.as_int; + break; + case PROP_TYPE_REAL: + prop_table = "edge_props_real"; + val_bind.type = BIND_DOUBLE; + val_bind.value.real = pv.as_real; + break; + case PROP_TYPE_BOOLEAN: + prop_table = "edge_props_bool"; + val_bind.type = BIND_INT; + val_bind.value.integer = pv.as_bool ? 1 : 0; + break; + default: + break; + } + property_value_free(&pv); + if (!prop_table) continue; + offset += snprintf(sql + offset, sizeof(sql) - offset, + " AND EXISTS (SELECT 1 FROM %s ep%d" + " JOIN property_keys pk%d ON ep%d.key_id = pk%d.id" + " WHERE ep%d.edge_id = e.id AND pk%d.key = ? AND ep%d.value = ?)", + prop_table, i, i, i, i, i, i, i); + bindings[bind_count].type = BIND_TEXT; + bindings[bind_count].value.text = pair->key; + bind_count++; + bindings[bind_count] = val_bind; + bind_count++; + } else if (pair->key && pair->value && pair->value->type == AST_NODE_LITERAL) { cypher_literal *lit = (cypher_literal*)pair->value; if (bind_count + 2 > MAX_BINDINGS) break; @@ -385,12 +564,14 @@ int find_edge_by_pattern(cypher_executor *executor, int source_id, int target_id int rc = sqlite3_prepare_v2(executor->db, sql, -1, &stmt, NULL); if (rc != SQLITE_OK) { CYPHER_DEBUG("MERGE find edge query prepare failed: %s", sqlite3_errmsg(executor->db)); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return -1; } if (bind_all_params(stmt, bindings, bind_count) != 0) { CYPHER_DEBUG("MERGE find edge query bind failed"); sqlite3_finalize(stmt); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return -1; } @@ -401,6 +582,7 @@ int find_edge_by_pattern(cypher_executor *executor, int source_id, int target_id } sqlite3_finalize(stmt); + for (int k = 0; k < owned_count; k++) free(owned_strs[k]); return edge_id; } /* Execute MERGE clause — canonical signature (I-0041 C8–C10). @@ -562,6 +744,10 @@ int execute_merge_clause(cypher_executor *executor, cypher_merge *merge, cypher_schema_set_node_property(executor->schema_mgr, node_id, pair->key, prop_type, prop_value); result->properties_set++; } + } else if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER) { + set_node_prop_from_param(executor, node_id, pair->key, + (cypher_parameter*)pair->value, result); } } } @@ -685,6 +871,10 @@ int execute_merge_clause(cypher_executor *executor, cypher_merge *merge, cypher_schema_set_node_property(executor->schema_mgr, target_node_id, pair->key, prop_type, prop_value); result->properties_set++; } + } else if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER) { + set_node_prop_from_param(executor, target_node_id, pair->key, + (cypher_parameter*)pair->value, result); } } } @@ -1214,6 +1404,10 @@ int execute_match_merge_query_with_varmap(cypher_executor *executor, cypher_matc cypher_schema_set_node_property(executor->schema_mgr, node_id, pair->key, prop_type, prop_value); result->properties_set++; } + } else if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER) { + set_node_prop_from_param(executor, node_id, pair->key, + (cypher_parameter*)pair->value, result); } } } @@ -1338,6 +1532,10 @@ int execute_match_merge_query_with_varmap(cypher_executor *executor, cypher_matc cypher_schema_set_node_property(executor->schema_mgr, target_node_id, pair->key, prop_type, prop_value); result->properties_set++; } + } else if (pair->key && pair->value && + pair->value->type == AST_NODE_PARAMETER) { + set_node_prop_from_param(executor, target_node_id, pair->key, + (cypher_parameter*)pair->value, result); } } } diff --git a/tests/test_executor_merge.c b/tests/test_executor_merge.c index 4037630c..7ee427da 100644 --- a/tests/test_executor_merge.c +++ b/tests/test_executor_merge.c @@ -881,6 +881,82 @@ static void test_merge_with_edge_variable_return(void) } } +/* GitHub #97 prerequisite: MERGE must honor parameter-valued inline + * properties both when matching and when creating. Previously they were + * silently ignored: MERGE (n:L {id: $id}) matched any :L node and created + * nodes without the property; MERGE (a)-[r:T {id: $eid}]->(b) matched any + * existing edge on the triple, so parallel edges could not be addressed. */ +static void test_merge_param_properties(void) +{ + cypher_executor *executor = cypher_executor_create(test_db); + CU_ASSERT_PTR_NOT_NULL(executor); + if (!executor) return; + + /* Node MERGE with a parameter property: create sets the property... */ + cypher_result *r = cypher_executor_execute_params(executor, + "MERGE (n:ParamMerge {pid: $p})", "{\"p\": \"m1\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->nodes_created, 1); cypher_result_free(r); } + + /* ...re-merge with the same value matches (no new node)... */ + r = cypher_executor_execute_params(executor, + "MERGE (n:ParamMerge {pid: $p})", "{\"p\": \"m1\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->nodes_created, 0); cypher_result_free(r); } + + /* ...and a different value creates a second node. */ + r = cypher_executor_execute_params(executor, + "MERGE (n:ParamMerge {pid: $p})", "{\"p\": \"m2\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->nodes_created, 1); cypher_result_free(r); } + + /* The created nodes carry the property value. */ + r = cypher_executor_execute(executor, + "MATCH (n:ParamMerge {pid: 'm2'}) RETURN n.pid"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { + CU_ASSERT_TRUE(r->success); + CU_ASSERT_EQUAL(r->row_count, 1); + cypher_result_free(r); + } + + /* Edge MERGE with a parameter property: distinct ids create parallel + * edges on the same (source, target, type) triple; repeats match. */ + r = cypher_executor_execute_params(executor, + "MATCH (a:ParamMerge {pid: $s}), (b:ParamMerge {pid: $t}) " + "MERGE (a)-[e:PM_REL {eid: $e}]->(b)", + "{\"s\": \"m1\", \"t\": \"m2\", \"e\": \"e1\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->relationships_created, 1); cypher_result_free(r); } + + r = cypher_executor_execute_params(executor, + "MATCH (a:ParamMerge {pid: $s}), (b:ParamMerge {pid: $t}) " + "MERGE (a)-[e:PM_REL {eid: $e}]->(b)", + "{\"s\": \"m1\", \"t\": \"m2\", \"e\": \"e1\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->relationships_created, 0); cypher_result_free(r); } + + r = cypher_executor_execute_params(executor, + "MATCH (a:ParamMerge {pid: $s}), (b:ParamMerge {pid: $t}) " + "MERGE (a)-[e:PM_REL {eid: $e}]->(b)", + "{\"s\": \"m1\", \"t\": \"m2\", \"e\": \"e2\"}"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { CU_ASSERT_TRUE(r->success); CU_ASSERT_EQUAL(r->relationships_created, 1); cypher_result_free(r); } + + r = cypher_executor_execute(executor, + "MATCH ()-[e:PM_REL]->() RETURN count(e) AS c"); + CU_ASSERT_PTR_NOT_NULL(r); + if (r) { + CU_ASSERT_TRUE(r->success); + if (r->success && r->row_count == 1 && r->data[0][0]) { + CU_ASSERT_STRING_EQUAL(r->data[0][0], "2"); + } + cypher_result_free(r); + } + + cypher_executor_free(executor); +} + /* Initialize the MERGE executor test suite */ int init_executor_merge_suite(void) { @@ -908,7 +984,8 @@ int init_executor_merge_suite(void) !CU_add_test(suite, "MERGE+WITH+SET no RETURN", test_merge_with_set_no_return) || !CU_add_test(suite, "MERGE+WITH+RETURN no SET", test_merge_with_return_no_set) || !CU_add_test(suite, "MERGE+WITH+multi-SET", test_merge_with_multiple_set) || - !CU_add_test(suite, "MERGE+WITH+edge variable", test_merge_with_edge_variable_return)) { + !CU_add_test(suite, "MERGE+WITH+edge variable", test_merge_with_edge_variable_return) || + !CU_add_test(suite, "MERGE with parameter properties", test_merge_param_properties)) { return CU_get_error(); } From f17067886bf0c3037a30a20c2f1c3e8a7895c2de Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:59:53 +0000 Subject: [PATCH 4/5] docs: state concurrent multi-process access inherits SQLite's guarantees Requested in #95: neither the README nor the docs mentioned concurrent writers, multi-process access, or WAL mode, leaving it an open question for anyone evaluating embedded concurrent-write use. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QuzbsuTddFm245sZ9egekS --- docs/src/explanation/architecture.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/src/explanation/architecture.md b/docs/src/explanation/architecture.md index 19347c16..aa3ebe82 100644 --- a/docs/src/explanation/architecture.md +++ b/docs/src/explanation/architecture.md @@ -11,6 +11,7 @@ Building a purpose-built graph engine would require implementing disk layout, bu The transpiler approach means: - **Durability and atomicity come for free.** Every write goes through SQLite's WAL and journalling machinery. +- **Concurrent access inherits SQLite's guarantees.** Multiple processes (not just threads) can safely read and write the same database file at the same time; SQLite's locking serialises the writes with no corruption or lost updates. The default rollback-journal mode allows one writer at a time with readers blocked during writes; enabling [WAL mode](https://www.sqlite.org/wal.html) (`PRAGMA journal_mode=WAL`) lets readers proceed concurrently with a writer and typically improves write throughput. GraphQLite adds no locking of its own — whatever concurrency SQLite supports in your configuration is what you get. - **Standard tooling works.** The underlying tables are plain SQLite tables. You can inspect them with the SQLite CLI, use SQLite backup APIs, and attach the database to other tools. - **Query execution is handled by a proven optimiser.** The generated SQL benefits from SQLite's query planner, covering indexes, and prepared statement caching. From 31b82df6a62b0e0ed9b9f1a7cb8d23f65e16ed88 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 03:03:33 +0000 Subject: [PATCH 5/5] tests: functional regressions + coverage matrix for GH-95/96/97 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests/functional/39_issue_regression_tests.sql (the file the semantic-coverage-matrix process points at) with hard assertions — a CHECK-constrained temp table aborts the run under sqlite3 -bail on any mismatch — covering all three fixes, and links the cells from docs/testing/semantic-coverage-matrix.md. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QuzbsuTddFm245sZ9egekS --- docs/testing/semantic-coverage-matrix.md | 36 ++++- .../functional/39_issue_regression_tests.sql | 132 ++++++++++++++++++ tests/functional/README.md | 5 + 3 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 tests/functional/39_issue_regression_tests.sql diff --git a/docs/testing/semantic-coverage-matrix.md b/docs/testing/semantic-coverage-matrix.md index 224e3e7b..8c10550f 100644 --- a/docs/testing/semantic-coverage-matrix.md +++ b/docs/testing/semantic-coverage-matrix.md @@ -55,7 +55,7 @@ out in sections 2 and 3. | `MERGE (a)-[r:R {k:v}]->(b)` | rel | `39:T-0187` ✓ | `39:T-0187` ✓ (T-0186/7) | GAP | n/a | | `MERGE (n) SET n.k = v` | node | `39:T-0195a` ✓ | GAP | GAP | n/a | | `MERGE (n) SET n += {..}` | node | `39:T-0195b` ✓ | GAP | GAP | `39:T-0195b` ✓ | -| `MATCH (a) CREATE (a)-[:R]->(b)` | new rel | `10:…` ✓ | GAP | GAP | n/a | +| `MATCH (a) CREATE (a)-[:R]->(b)` | new rel | `10:…`, `39:GH-95` ✓ (incl. `RETURN r`/`r.k` read-back) | GAP | GAP | n/a | | `MATCH (a) CREATE (a)-[:R]->(b) SET b.k = v` | new node | `39:T-0198c` ✓ | GAP | GAP | n/a | | `MATCH (a) MATCH (b) CREATE (a)-[:R]->(b)` | rel | `39:T-0197a` ✓ | `39:T-0197c` ✓ | GAP | n/a | | `MATCH (a) MATCH (b) MERGE (a)-[r]->(b) SET r.k = v` | rel | `39:T-0196` ✓ | `39:T-0196` ✓ | GAP | n/a | @@ -108,6 +108,9 @@ shipped — remaining gaps below. | Trailing SET after MERGE, rel | ✓ | GAP | file follow-up | | Trailing SET after MATCH+MERGE, rel | ✓ | GAP | file follow-up | | `SET r +=` on rel var | `39:T-0202c` ✓ | `39:T-0202d` ✓ | | +| MATCH rel inline prop filter (read) | `39:GH-96 1.2` ✓ | `39:GH-96 1.1/1.4` ✓ | fixed in GH-96 | +| MERGE node inline prop (match phase) | ✓ | `39:GH-97 3.1` ✓ | fixed with GH-97 | +| MERGE rel inline prop (match phase) | ✓ | `39:GH-97 3.2` ✓ | fixed with GH-97 | --- @@ -699,3 +702,34 @@ WithOrderBy4 [12] ("Sort by an aliased aggregate projection") and Pattern2 [8] whole table. The catch-all branch now appends the (non-aggregate) transformed expression to GROUP BY, mirroring the identifier/property branches. Aggregate projections (`find_aggregating_call` non-null) are still excluded from GROUP BY. + +## Coverage update (2026-08-25) — GitHub issues #95/#96/#97 + +Regression tests live in `tests/functional/39_issue_regression_tests.sql` +(hard assertions: a CHECK-constrained temp table aborts the run under +`sqlite3 -bail` on any mismatch). Verified alongside unit 947/947, Python +357, Rust 244, and a full TCK pass-set diff (zero regressions). + +- **GH-96 — `MATCH ()-[r:T {k: $param}]->()`** — relationship inline + property filters previously *skipped* parameter values entirely, + matching every edge of the type (node patterns and WHERE clauses were + unaffected). `transform_match.c` now emits the same OR-of-EXISTS + parameter condition the node path uses. Cells: read-filter literal vs + `$param` symmetry for rel patterns (section 3), including a + `SET`-scoped-by-filter safety check. +- **GH-95 — `MATCH … CREATE (a)-[r]->(b) RETURN r`** — RETURN of a + variable introduced by CREATE (not bound by any MATCH) raised + "Unknown variable" *after* committing the write. The + MATCH+CREATE+RETURN handler now projects such variables from per-row + variable maps (bare var, `var.k`, aggregates, SKIP/LIMIT; one result + row per MATCH row). Cell: `MATCH (a) CREATE (a)-[:R]->(b)` read-back + on the created rel var (section 1). +- **GH-97 — MERGE `$param` inline properties** — the MERGE match phase + ignored parameter-valued inline properties for both nodes and edges + (matching any node of the label / any edge on the triple), and node + creation dropped them. `executor_merge.c` now resolves parameters in + `find_node_by_pattern`, `find_edge_by_pattern`, and the node-create + property phase. This makes caller-assigned edge ids workable: + `upsert_edge(..., edge_id=...)` (Python) / `upsert_edge_with_id` + (Rust) merge on an `id` relationship property so parallel edges on + the same (source, target, type) triple are individually addressable. diff --git a/tests/functional/39_issue_regression_tests.sql b/tests/functional/39_issue_regression_tests.sql new file mode 100644 index 00000000..f9614efc --- /dev/null +++ b/tests/functional/39_issue_regression_tests.sql @@ -0,0 +1,132 @@ +-- ======================================================================== +-- Test 39: Issue Regression Tests +-- ======================================================================== +-- PURPOSE: Round-trip regression tests for reported GitHub issues, one +-- section per issue. Referenced from +-- docs/testing/semantic-coverage-matrix.md. +-- COVERS: GH-96 (rel inline property filter with $param), +-- GH-95 (MATCH+CREATE ... RETURN created rel var), +-- GH-97 (MERGE with $param inline properties / parallel edges) +-- NOTE: Assertions are hard: _assert has CHECK (ok = 1), so under +-- `sqlite3 -bail` any failed expectation aborts the run. +-- ======================================================================== + +.load ./build/graphqlite + +SELECT '=== Test 39: Issue Regression Tests ===' as test_section; + +CREATE TEMP TABLE _assert(name TEXT, ok INTEGER CHECK (ok = 1)); + +-- ======================================================================= +-- SECTION 1: GitHub #96 — inline relationship property filter with $param +-- ======================================================================= +SELECT '=== Section 1: GH-96 rel inline property filter with $param ===' as section; + +SELECT cypher('CREATE (:G96 {name: "alice"})') as setup; +SELECT cypher('CREATE (:G96 {name: "bob"})') as setup; +SELECT cypher('MATCH (x:G96 {name:"alice"}),(y:G96 {name:"bob"}) CREATE (x)-[:G96_KNOWS {w: 1}]->(y)') as setup; +SELECT cypher('MATCH (x:G96 {name:"alice"}),(y:G96 {name:"bob"}) CREATE (x)-[:G96_KNOWS {w: 2}]->(y)') as setup; +SELECT cypher('MATCH (x:G96 {name:"alice"}),(y:G96 {name:"bob"}) CREATE (x)-[:G96_KNOWS {w: 3, tag: "target"}]->(y)') as setup; + +SELECT 'Test 1.1 - inline string $param matches only the tagged edge:' as test_name; +SELECT cypher('MATCH ()-[r:G96_KNOWS {tag: $t}]->() RETURN r.w AS w', '{"t": "target"}') as result; +INSERT INTO _assert SELECT 'GH-96 1.1 rows', + json_array_length(cypher('MATCH ()-[r:G96_KNOWS {tag: $t}]->() RETURN r.w AS w', '{"t": "target"}')) = 1; +INSERT INTO _assert SELECT 'GH-96 1.1 value', + json_extract(cypher('MATCH ()-[r:G96_KNOWS {tag: $t}]->() RETURN r.w AS w', '{"t": "target"}'), '$[0].w') = 3; + +SELECT 'Test 1.2 - inline literal filter agrees with the $param filter:' as test_name; +INSERT INTO _assert SELECT 'GH-96 1.2 literal parity', + json_array_length(cypher('MATCH ()-[r:G96_KNOWS {tag: "target"}]->() RETURN r.w AS w')) = 1; + +SELECT 'Test 1.3 - WHERE-clause $param agrees with the inline filter:' as test_name; +INSERT INTO _assert SELECT 'GH-96 1.3 where parity', + json_array_length(cypher('MATCH ()-[r:G96_KNOWS]->() WHERE r.tag = $t RETURN r.w AS w', '{"t": "target"}')) = 1; + +SELECT 'Test 1.4 - inline integer $param:' as test_name; +INSERT INTO _assert SELECT 'GH-96 1.4 int param', + json_extract(cypher('MATCH ()-[r:G96_KNOWS {w: $v}]->() RETURN r.w AS w', '{"v": 2}'), '$[0].w') = 2; + +SELECT 'Test 1.5 - non-matching $param returns no rows:' as test_name; +INSERT INTO _assert SELECT 'GH-96 1.5 no match', + json_array_length(cypher('MATCH ()-[r:G96_KNOWS {tag: $t}]->() RETURN r.w AS w', '{"t": "nope"}')) = 0; + +SELECT 'Test 1.6 - SET scoped by inline $param filter touches one edge only:' as test_name; +SELECT cypher('MATCH ()-[r:G96_KNOWS {tag: $t}]->() SET r.marked = 1', '{"t": "target"}') as result; +INSERT INTO _assert SELECT 'GH-96 1.6 scoped set', + json_array_length(cypher('MATCH ()-[r:G96_KNOWS]->() WHERE r.marked = 1 RETURN r.w AS w')) = 1; + +-- ======================================================================= +-- SECTION 2: GitHub #95 — RETURN of rel created between MATCH-bound nodes +-- ======================================================================= +SELECT '=== Section 2: GH-95 MATCH+CREATE ... RETURN created rel var ===' as section; + +SELECT cypher('CREATE (:G95Hub {name: "hub0"})') as setup; +SELECT cypher('CREATE (:G95Hub {name: "hub1"})') as setup; + +SELECT 'Test 2.1 - comma multi-pattern MATCH, RETURN r.prop:' as test_name; +SELECT cypher('MATCH (x:G95Hub {name: "hub0"}), (y:G95Hub {name: "hub1"}) CREATE (x)-[r:G95_LINKS {seq: 1}]->(y) RETURN r.seq AS s') as result; +INSERT INTO _assert SELECT 'GH-95 2.1 value', + json_extract(cypher('MATCH ()-[e:G95_LINKS]->() RETURN count(e) AS c'), '$[0].c') = 1; + +SELECT 'Test 2.2 - two separate MATCH clauses, RETURN r.prop:' as test_name; +INSERT INTO _assert SELECT 'GH-95 2.2 returned value', + json_extract(cypher('MATCH (x:G95Hub {name: "hub0"}) MATCH (y:G95Hub {name: "hub1"}) CREATE (x)-[r2:G95_LINKS {seq: 2}]->(y) RETURN r2.seq AS s'), '$[0].s') = 2; + +SELECT 'Test 2.3 - bare RETURN r carries the relationship type:' as test_name; +INSERT INTO _assert SELECT 'GH-95 2.3 bare rel', + json_extract(cypher('MATCH (x:G95Hub {name: "hub0"}), (y:G95Hub {name: "hub1"}) CREATE (x)-[r:G95_LINKS2 {seq: 3}]->(y) RETURN r'), '$[0].r.type') = 'G95_LINKS2'; + +SELECT 'Test 2.4 - exactly one edge per CREATE (no duplicate writes):' as test_name; +INSERT INTO _assert SELECT 'GH-95 2.4 edge count', + json_extract(cypher('MATCH ()-[e:G95_LINKS]->() RETURN count(e) AS c'), '$[0].c') = 2; + +SELECT 'Test 2.5 - multi-row MATCH creates and returns one row per match:' as test_name; +SELECT cypher('CREATE (:G95P {n: 1})') as setup; +SELECT cypher('CREATE (:G95P {n: 2})') as setup; +SELECT cypher('CREATE (:G95P {n: 3})') as setup; +INSERT INTO _assert SELECT 'GH-95 2.5 rows', + json_array_length(cypher('MATCH (p:G95P) CREATE (p)-[r:G95_HAS {k: 7}]->(:G95Q) RETURN r.k AS k')) = 3; +INSERT INTO _assert SELECT 'GH-95 2.5 edges', + json_extract(cypher('MATCH ()-[e:G95_HAS]->() RETURN count(e) AS c'), '$[0].c') = 3; + +-- ======================================================================= +-- SECTION 3: GitHub #97 — MERGE with $param inline props / parallel edges +-- ======================================================================= +SELECT '=== Section 3: GH-97 MERGE with $param inline properties ===' as section; + +SELECT 'Test 3.1 - node MERGE with $param creates then matches in place:' as test_name; +SELECT cypher('MERGE (n:G97 {pid: $p})', '{"p": "m1"}') as result; +SELECT cypher('MERGE (n:G97 {pid: $p})', '{"p": "m1"}') as result; +SELECT cypher('MERGE (n:G97 {pid: $p})', '{"p": "m2"}') as result; +INSERT INTO _assert SELECT 'GH-97 3.1 node count', + json_extract(cypher('MATCH (n:G97) RETURN count(n) AS c'), '$[0].c') = 2; +INSERT INTO _assert SELECT 'GH-97 3.1 prop stored', + json_array_length(cypher('MATCH (n:G97 {pid: "m2"}) RETURN n.pid AS p')) = 1; + +SELECT 'Test 3.2 - edge MERGE keyed on $param id addresses parallel edges:' as test_name; +SELECT cypher('MATCH (a:G97 {pid: $s}), (b:G97 {pid: $t}) MERGE (a)-[e:G97_REL {eid: $e}]->(b)', '{"s": "m1", "t": "m2", "e": "e1"}') as result; +SELECT cypher('MATCH (a:G97 {pid: $s}), (b:G97 {pid: $t}) MERGE (a)-[e:G97_REL {eid: $e}]->(b)', '{"s": "m1", "t": "m2", "e": "e1"}') as result; +SELECT cypher('MATCH (a:G97 {pid: $s}), (b:G97 {pid: $t}) MERGE (a)-[e:G97_REL {eid: $e}]->(b)', '{"s": "m1", "t": "m2", "e": "e2"}') as result; +INSERT INTO _assert SELECT 'GH-97 3.2 parallel edges', + json_extract(cypher('MATCH ()-[e:G97_REL]->() RETURN count(e) AS c'), '$[0].c') = 2; + +SELECT 'Test 3.3 - each parallel edge individually addressable by its id:' as test_name; +SELECT cypher('MATCH ()-[e:G97_REL {eid: $e}]->() SET e.seq = 10', '{"e": "e1"}') as result; +INSERT INTO _assert SELECT 'GH-97 3.3 targeted update', + json_array_length(cypher('MATCH ()-[e:G97_REL]->() WHERE e.seq = 10 RETURN e.eid AS eid')) = 1; + +-- ======================================================================= +-- VERIFICATION SUMMARY +-- ======================================================================= +SELECT '=== Assertions run (all must show ok=1) ===' as section; +SELECT name, ok FROM _assert; + +-- ======================================================================= +-- TEARDOWN +-- ======================================================================= +SELECT '=== Teardown: Cleaning up ===' as section; + +SELECT cypher('MATCH (n) DETACH DELETE n') as cleanup; + +SELECT '=== Test 39 Complete ===' as test_section; diff --git a/tests/functional/README.md b/tests/functional/README.md index 56de8fea..e99054a7 100644 --- a/tests/functional/README.md +++ b/tests/functional/README.md @@ -56,6 +56,11 @@ The functional tests are numbered sequentially and cover all aspects of GraphQLi **Covers**: Node reuse vs creation, relationship creation with existing nodes **Key Tests**: Node reuse verification, error handling for missing nodes, efficiency patterns +### 39_issue_regression_tests.sql +**Purpose**: Round-trip regression tests for reported GitHub issues (see `docs/testing/semantic-coverage-matrix.md`) +**Covers**: GH-96 (rel inline property filter with `$param`), GH-95 (MATCH+CREATE ... RETURN created rel var), GH-97 (MERGE with `$param` inline properties / parallel edges) +**Key Tests**: Hard assertions via a CHECK-constrained temp table — any mismatch aborts the run under `sqlite3 -bail` + ## Running Tests ### Run All Functional Tests