From 98ac97692ffbbbefa223ddb499e0e82b1b2bc659 Mon Sep 17 00:00:00 2001 From: Justin Kim Date: Thu, 30 Jul 2026 21:01:19 +0900 Subject: [PATCH] fix(columnar): retain canonical input relation names --- tests/test_wirelog_advanced.c | 158 ++++++++++++++++++++++++++++++++++ tests/test_wirelog_easy.c | 131 ++++++++++++++++++++++++++++ wirelog/columnar/internal.h | 8 +- wirelog/columnar/session.c | 24 +++--- 4 files changed, 308 insertions(+), 13 deletions(-) diff --git a/tests/test_wirelog_advanced.c b/tests/test_wirelog_advanced.c index 6e85570d..e17b363a 100644 --- a/tests/test_wirelog_advanced.c +++ b/tests/test_wirelog_advanced.c @@ -55,6 +55,11 @@ static const char *PROG_SRC = "path(X,Y) :- edge(X,Y).\n" "path(X,Z) :- path(X,Y), edge(Y,Z).\n"; +static const char *RELATION_NAME_LIFETIME_SRC + = ".decl edge(x: int64, y: int64)\n" + ".decl reach(x: int64, y: int64)\n" + "reach(X, Y) :- edge(X, Y).\n"; + static const char *PROG_RBAC_SRC = ".decl role_permission(role:symbol,perm:symbol)\n" ".decl member_of(user:symbol,role:symbol,scope:symbol)\n" @@ -382,6 +387,158 @@ test_insert_step_delta(void) return rc; } +/* Parity (#931): mirrors test_wirelog_easy.c::test_relation_name_lifetime. */ +static int +test_relation_name_lifetime(void) +{ + int rc = 0; + int64_t row[2] = { 1, 2 }; + wirelog_program_t *prog + = parse_or_die(RELATION_NAME_LIFETIME_SRC, "relation-name lifetime"); + if (!prog) + return 1; + + wirelog_session_t *s = NULL; + if (wirelog_session_create(prog, WIRELOG_BACKEND_DEFAULT, 1, &s) + != WIRELOG_OK) { + wirelog_program_free(prog); + return 1; + } + + struct tuple_filter tuples = { .target_relation = "reach" }; + if (wirelog_session_snapshot(s, filter_tuples, &tuples) != WIRELOG_OK) { + fprintf(stderr, "relation-name lifetime: baseline snapshot failed\n"); + rc = 1; + goto regular_out; + } + + char regular_relation[8] = "edge"; + if (wirelog_session_insert(s, regular_relation, row, 1, 2) + != WIRELOG_OK) { + fprintf(stderr, "relation-name lifetime: regular insert failed\n"); + rc = 1; + goto regular_out; + } + strcpy(regular_relation, "bogus"); + + memset(&tuples, 0, sizeof(tuples)); + tuples.target_relation = "reach"; + if (wirelog_session_snapshot(s, filter_tuples, &tuples) != WIRELOG_OK) { + fprintf(stderr, + "relation-name lifetime: post-insert snapshot failed\n"); + rc = 1; + goto regular_out; + } + bool found_snapshot = false; + for (int i = 0; i < tuples.count; i++) { + if (tuples.ncols[i] == 2 && tuples.rows[i][0] == row[0] + && tuples.rows[i][1] == row[1]) { + found_snapshot = true; + break; + } + } + if (!found_snapshot) { + fprintf(stderr, + "relation-name lifetime: mutable regular name suppressed " + "reach(1,2)\n"); + rc = 1; + } + +regular_out: + wirelog_session_destroy(s); + wirelog_program_free(prog); + if (rc != 0) + return rc; + + prog = parse_or_die( + RELATION_NAME_LIFETIME_SRC, "relation-name delta lifetime"); + if (!prog) + return 1; + s = NULL; + if (wirelog_session_create(prog, WIRELOG_BACKEND_DEFAULT, 1, &s) + != WIRELOG_OK) { + wirelog_program_free(prog); + return 1; + } + + struct delta_collector deltas; + memset(&deltas, 0, sizeof(deltas)); + if (wirelog_session_set_delta_cb(s, collect_delta_rows, &deltas) + != WIRELOG_OK) { + fprintf(stderr, "relation-name lifetime: set_delta_cb failed\n"); + rc = 1; + goto delta_out; + } + + char inserted_relation[8] = "edge"; + if (wirelog_session_insert(s, inserted_relation, row, 1, 2) + != WIRELOG_OK) { + fprintf(stderr, "relation-name lifetime: delta insert failed\n"); + rc = 1; + goto delta_out; + } + strcpy(inserted_relation, "bogus"); + if (wirelog_session_step(s) != WIRELOG_OK) { + fprintf(stderr, + "relation-name lifetime: post-insert step failed\n"); + rc = 1; + goto delta_out; + } + + bool found_insert = false; + for (int i = 0; i < deltas.count; i++) { + if (strcmp(deltas.relations[i], "reach") == 0 + && deltas.ncols[i] == 2 && deltas.rows[i][0] == row[0] + && deltas.rows[i][1] == row[1] && deltas.diffs[i] == 1) { + found_insert = true; + break; + } + } + int before_remove = deltas.count; + + char removed_relation[8] = "edge"; + if (wirelog_session_remove(s, removed_relation, row, 1, 2) + != WIRELOG_OK) { + fprintf(stderr, "relation-name lifetime: delta remove failed\n"); + rc = 1; + goto delta_out; + } + strcpy(removed_relation, "bogus"); + if (wirelog_session_step(s) != WIRELOG_OK) { + fprintf(stderr, + "relation-name lifetime: post-remove step failed\n"); + rc = 1; + goto delta_out; + } + + bool found_remove = false; + for (int i = before_remove; i < deltas.count; i++) { + if (strcmp(deltas.relations[i], "reach") == 0 + && deltas.ncols[i] == 2 && deltas.rows[i][0] == row[0] + && deltas.rows[i][1] == row[1] && deltas.diffs[i] == -1) { + found_remove = true; + break; + } + } + if (!found_insert) { + fprintf(stderr, + "relation-name lifetime: mutable inserted name suppressed " + "+reach(1,2)\n"); + rc = 1; + } + if (!found_remove) { + fprintf(stderr, + "relation-name lifetime: mutable removed name suppressed " + "-reach(1,2)\n"); + rc = 1; + } + +delta_out: + wirelog_session_destroy(s); + wirelog_program_free(prog); + return rc; +} + /* T6: insert / remove pair leaves the relation back at zero. */ static int test_insert_remove_roundtrip(void) @@ -1617,6 +1774,7 @@ main(void) failures += test_create_invalid_backend(); failures += test_inline_facts_seeded(); failures += test_insert_step_delta(); + failures += test_relation_name_lifetime(); failures += test_insert_remove_roundtrip(); failures += test_null_safety(); failures += test_open_parse_error(); diff --git a/tests/test_wirelog_easy.c b/tests/test_wirelog_easy.c index 0fbeb63b..2f719f19 100644 --- a/tests/test_wirelog_easy.c +++ b/tests/test_wirelog_easy.c @@ -78,6 +78,11 @@ static const char *ACCESS_CONTROL_SRC ".decl granted(user: symbol, perm: symbol)\n" "granted(U, P) :- can(U, P).\n"; +static const char *RELATION_NAME_LIFETIME_SRC + = ".decl edge(x: int64, y: int64)\n" + ".decl reach(x: int64, y: int64)\n" + "reach(X, Y) :- edge(X, Y).\n"; + /* ======================================================================== */ /* Delta Collector */ /* ======================================================================== */ @@ -622,6 +627,131 @@ test_eager_sessions_preintern_projection_literals(void) PASS(); } +/* PARITY: paired in test_wirelog_advanced.c by the same test name (#931). */ +static void +test_relation_name_lifetime(void) +{ + TEST("insert/remove retain relation name until evaluation"); + + int64_t row[2] = { 1, 2 }; + wirelog_easy_session_t *s = NULL; + if (wirelog_easy_open(RELATION_NAME_LIFETIME_SRC, &s) != WIRELOG_OK + || !s) { + FAIL("regular-path open failed"); + return; + } + + tuple_collector_t tuples; + memset(&tuples, 0, sizeof(tuples)); + if (wirelog_easy_snapshot(s, "reach", collect_tuple, &tuples) + != WIRELOG_OK) { + FAIL("baseline snapshot failed"); + wirelog_easy_close(s); + return; + } + + char regular_relation[8] = "edge"; + if (wirelog_easy_insert(s, regular_relation, row, 2) != WIRELOG_OK) { + FAIL("regular-path insert failed"); + wirelog_easy_close(s); + return; + } + strcpy(regular_relation, "bogus"); + + memset(&tuples, 0, sizeof(tuples)); + if (wirelog_easy_snapshot(s, "reach", collect_tuple, &tuples) + != WIRELOG_OK) { + FAIL("snapshot after mutable-name insert failed"); + wirelog_easy_close(s); + return; + } + bool found_snapshot = false; + for (int i = 0; i < tuples.count; i++) { + if (strcmp(tuples.relations[i], "reach") == 0 + && tuples.ncols[i] == 2 && tuples.rows[i][0] == row[0] + && tuples.rows[i][1] == row[1]) { + found_snapshot = true; + break; + } + } + wirelog_easy_close(s); + if (!found_snapshot) { + FAIL("mutable regular relation name suppressed reach(1,2)"); + return; + } + + s = NULL; + if (wirelog_easy_open(RELATION_NAME_LIFETIME_SRC, &s) != WIRELOG_OK + || !s) { + FAIL("delta-path open failed"); + return; + } + delta_collector_t deltas; + memset(&deltas, 0, sizeof(deltas)); + if (wirelog_easy_set_delta_cb(s, collect_delta, &deltas) != WIRELOG_OK) { + FAIL("set_delta_cb failed"); + wirelog_easy_close(s); + return; + } + + char inserted_relation[8] = "edge"; + if (wirelog_easy_insert(s, inserted_relation, row, 2) != WIRELOG_OK) { + FAIL("delta-path insert failed"); + wirelog_easy_close(s); + return; + } + strcpy(inserted_relation, "bogus"); + if (wirelog_easy_step(s) != WIRELOG_OK) { + FAIL("step after mutable-name insert failed"); + wirelog_easy_close(s); + return; + } + + bool found_insert = false; + for (int i = 0; i < deltas.count; i++) { + if (strcmp(deltas.relations[i], "reach") == 0 + && deltas.ncols[i] == 2 && deltas.rows[i][0] == row[0] + && deltas.rows[i][1] == row[1] && deltas.diffs[i] == 1) { + found_insert = true; + break; + } + } + int before_remove = deltas.count; + + char removed_relation[8] = "edge"; + if (wirelog_easy_remove(s, removed_relation, row, 2) != WIRELOG_OK) { + FAIL("delta-path remove failed"); + wirelog_easy_close(s); + return; + } + strcpy(removed_relation, "bogus"); + if (wirelog_easy_step(s) != WIRELOG_OK) { + FAIL("step after mutable-name remove failed"); + wirelog_easy_close(s); + return; + } + + bool found_remove = false; + for (int i = before_remove; i < deltas.count; i++) { + if (strcmp(deltas.relations[i], "reach") == 0 + && deltas.ncols[i] == 2 && deltas.rows[i][0] == row[0] + && deltas.rows[i][1] == row[1] && deltas.diffs[i] == -1) { + found_remove = true; + break; + } + } + wirelog_easy_close(s); + if (!found_insert) { + FAIL("mutable inserted relation name suppressed +reach(1,2)"); + return; + } + if (!found_remove) { + FAIL("mutable removed relation name suppressed -reach(1,2)"); + return; + } + PASS(); +} + static void test_inline_compound_body_binding(void) { @@ -1999,6 +2129,7 @@ main(void) test_intern_returns_same_id(); test_insert_step_delta(); test_eager_sessions_preintern_projection_literals(); + test_relation_name_lifetime(); test_inline_compound_body_binding(); test_inline_compound_body_join_binding(); test_inline_compound_functor_mismatch_is_empty(); diff --git a/wirelog/columnar/internal.h b/wirelog/columnar/internal.h index b387d1cd..97d5aaf7 100644 --- a/wirelog/columnar/internal.h +++ b/wirelog/columnar/internal.h @@ -886,7 +886,9 @@ typedef struct wl_col_session_t { bool tdd_decision_tracking_active; /* Phase 4: tracks which relation was just inserted via * col_session_insert_incremental, enables affected-stratum skip - * optimization. Borrowed pointer; lifetime: until next session_step. + * optimization. Session-owned canonical relation name; base and + * compound-side relations remain registered while input work is pending, + * so the pointer stays valid until a successful step/snapshot clears it. * NULL when no incremental insert preceded the current step (all strata * evaluated normally via affected_mask = UINT64_MAX). */ const char *last_inserted_relation; @@ -938,7 +940,9 @@ typedef struct wl_col_session_t { bool stratum_is_monotone[MAX_STRATA]; /* Retraction delta tracking (Issue #158): tracks which relation was just * removed via col_session_remove_incremental, enables delta retraction path. - * Borrowed pointer; lifetime: until next session_step. + * Session-owned canonical relation name; the base relation remains + * registered while retraction work is pending, so the pointer stays valid + * until a successful step clears it. * NULL when no incremental remove preceded the current step. */ const char *last_removed_relation; /* Retraction-seeded incremental evaluation (Issue #158). diff --git a/wirelog/columnar/session.c b/wirelog/columnar/session.c index f9c00644..ebe6f604 100644 --- a/wirelog/columnar/session.c +++ b/wirelog/columnar/session.c @@ -121,16 +121,18 @@ session_invalidate_relation_caches(wl_col_session_t *sess, const char *name) } static void -session_note_inserted_input(wl_col_session_t *sess, const char *relation, +session_note_inserted_input(wl_col_session_t *sess, const col_rel_t *relation, bool advance_epoch) { - session_invalidate_relation_caches(sess, relation); + const char *relation_name = relation->name; + + session_invalidate_relation_caches(sess, relation_name); if (advance_epoch) sess->outer_epoch++; if (sess->last_inserted_relation - && strcmp(sess->last_inserted_relation, relation) != 0) + && strcmp(sess->last_inserted_relation, relation_name) != 0) sess->pending_full_input_eval = true; - sess->last_inserted_relation = relation; + sess->last_inserted_relation = relation_name; sess->pending_input_change = true; sess->snapshot_stable_valid = false; } @@ -1682,7 +1684,7 @@ col_session_insert(wl_session_t *session, const char *relation, return rc; } - session_note_inserted_input(sess, relation, false); + session_note_inserted_input(sess, r, false); /* The non-incremental API must force a full epoch evaluation even when * the previous update targeted the same relation. */ sess->pending_full_input_eval = true; @@ -1773,7 +1775,7 @@ col_session_make_compound(wl_session_t *session, const char *functor, return rc; } - session_note_inserted_input(sess, side_rel->name, true); + session_note_inserted_input(sess, side_rel, true); *handle_out = handle; return 0; } @@ -1830,7 +1832,7 @@ col_session_insert_incremental(wl_session_t *session, const char *relation, } wl_col_session_t *sess = COL_SESSION(session); - session_note_inserted_input(sess, relation, true); + session_note_inserted_input(sess, r, true); return 0; } @@ -1890,7 +1892,7 @@ col_session_remove(wl_session_t *session, const char *relation, r->nrows = out_r; next_del:; } - session_invalidate_relation_caches(sess, relation); + session_invalidate_relation_caches(sess, r->name); sess->pending_input_change = true; sess->snapshot_stable_valid = false; return 0; @@ -1930,7 +1932,7 @@ col_session_remove_incremental(wl_session_t *session, const char *relation, /* Allocate $r$ delta relation to collect removed rows */ char rname[256]; - snprintf(rname, sizeof(rname), "$r$%s", relation); + snprintf(rname, sizeof(rname), "$r$%s", r->name); col_rel_t *rdelta = col_rel_new_auto(rname, num_cols); if (!rdelta) @@ -2004,10 +2006,10 @@ next_del_incr:; * so subsequent re-evaluation rebuilds hash indices without the removed * rows. Without this, cached arrangements contain stale entries that * produce phantom join matches during full re-eval retraction. */ - session_invalidate_relation_caches(sess, relation); + session_invalidate_relation_caches(sess, r->name); /* Mark removal for affected-stratum calculation */ - sess->last_removed_relation = relation; + sess->last_removed_relation = r->name; sess->outer_epoch++; sess->pending_input_change = true;