From 7cb4bcf0c3ebace0e458b0f0a0b8ad560a7bcfc7 Mon Sep 17 00:00:00 2001 From: "shoney.arickathil" Date: Sun, 30 Aug 2026 08:22:25 +0200 Subject: [PATCH] feat(db2-delta): keys-resident updates append, indexes follow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - table.c: wo_row_update_field/_slot no longer refuse `resident: keys`, both converge on one new static row_apply_field_keys - borrows (folds), shadow-checks uniqueness, appends the delta with the row's current offset as back-pointer, commits, THEN idx_remove_row + idx_add_row + wo_row_set_offset — failure through commit leaves the row's offset and index untouched - nv decoded to a VM value before touching the materialised copy, since wo_row_release drops every slot through the runtime, not db_val_free - borrow released on every exit, including every failure arm - resident: all path (row_apply_field_slot) byte-for-byte unchanged; wal.c untouched — Tasks 1/2 already expose everything needed - test_wal.c: plain field update read back, and an indexed scalar column updated then found via wo_idx_probe by its new value, gone from its old — both verified failing pre-implementation, passing after - concern: idx_hash/idx_cols_equal/wo_idx_probe cast Text slots to db_text* unconditionally; a keys-resident borrow decodes Text to a VM wo_str* (different layout) — pre-existing, left untouched; tests use a scalar index to sidestep it Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 89c56a13ed9f4abd082bfcabb46f46bb53d39faa) --- database/src/table.c | 152 +++++++++++++++++++++++++++++++++++----- runtime/test/test_wal.c | 127 +++++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+), 17 deletions(-) diff --git a/database/src/table.c b/database/src/table.c index d1df97d..604d0a4 100644 --- a/database/src/table.c +++ b/database/src/table.c @@ -988,20 +988,34 @@ static int row_apply_field_slot(wo_db *db, db_table *t, const wo_classdesc *c, db_row *r, uint32_t class_id, uint64_t id, uint32_t field, uint64_t nv, const char **msg, int *err_kind); +static int row_apply_field_keys(wo_db *db, uint32_t class_id, uint64_t id, + uint32_t field, uint64_t nv, const char **msg, + int *err_kind); int wo_row_update_field(wo_db *db, uint32_t class_id, uint64_t id, uint32_t field, uint64_t vm_val, const char **msg, int *err_kind) { if (err_kind) *err_kind = DB_ERR_MISC; - /* databasev2 2 (5d): a keys-resident row lives in the LOG, so there is no - * slab slot to mutate — writing into the borrow's scratch would discard - * the update silently, which is the one failure mode this iteration must - * not ship. Updating such a row means read-modify-APPEND (a new record, - * then re-point the offset), and that is not built yet. Refuse loudly. - * The loader refuses `resident: keys` outright, so this is defence in - * depth and a marker for the next implementer. */ + /* databasev2 3 (keys-resident delta updates): a keys-resident row lives + * in the LOG, so there is no slab slot to mutate — row_apply_field_keys + * does read-modify-APPEND instead of a slot swap. wo_row_offset1 is the + * cheap existence check wo_row_ptr would otherwise give us. */ if (wo_table_is_keys_resident(db, class_id)) { - *msg = "update on a `resident: keys` table is not implemented"; - return -1; + if (!wo_row_offset1(db, class_id, id)) { + *msg = "no such row"; + return -1; + } + const wo_classdesc *kc = &db->classes[class_id]; + if (field >= kc->field_cnt) { + *msg = "no such field"; + return -1; + } + int kok = 1; + uint64_t knv = db_val_encode(db->classes, kc->kinds[field], vm_val, &kok, msg); + if (!kok) { + if (err_kind) *err_kind = DB_ERR_BADKIND; + return -1; + } + return row_apply_field_keys(db, class_id, id, field, knv, msg, err_kind); } db_row *r = wo_row_ptr(db, class_id, id); if (!r) { @@ -1100,17 +1114,117 @@ static int row_apply_field_slot(wo_db *db, db_table *t, const wo_classdesc *c, return 0; } +/* keys-resident counterpart of row_apply_field_slot. There is no slab slot + * to swap — the row lives in the log — so the shape is read-modify-APPEND: + * borrow (folds), append a delta with the row's current offset as the + * back-pointer, commit, THEN move the id map to the new record. [nv] is + * already engine-encoded (same convention as row_apply_field_slot); + * consumed on every path. + * + * The borrow's materialised row holds VM values (wo_row_release drops every + * slot through the runtime), so [nv] is decoded to a VM value up front and + * that is what ever lands in r->slots[field] — the engine encoding is used + * only for the WAL record and freed once logged. + * + * Ordering: the unique shadow-check (against a shadow of the row, mirroring + * row_apply_field_slot's promise that a rejected update leaves the row + * untouched) and the WAL append+commit both happen BEFORE either the index + * or the id map move — so a failure at any point up to and including the + * commit leaves the live row (offset AND index) exactly as it was. Only a + * successful, durable commit is followed by the index swap and the offset + * repoint, which — being pure RAM bookkeeping a replay rebuilds from the log + * regardless — cannot itself meaningfully "fail" once reached. */ +static int row_apply_field_keys(wo_db *db, uint32_t class_id, uint64_t id, + uint32_t field, uint64_t nv, const char **msg, + int *err_kind) { + const wo_classdesc *c = &db->classes[class_id]; + db_table *t = &db->tables[class_id]; + const char *bmsg = "no such row"; + db_row *r = wo_row_borrow(db, class_id, id, &bmsg); + if (!r) { + db_val_free(c->kinds[field], nv); + *msg = bmsg; + return -1; + } + /* successful borrow proves db->rt and db->rt->wal are both set */ + uint64_t back_off = wo_row_offset1(db, class_id, id) - 1; + + int ok = 1; + uint64_t nv_vm = wo_val_decode_vm(db, db->rt, c->kinds[field], nv, &ok, msg); + if (!ok) { + wo_row_release(db, class_id, r); + db_val_free(c->kinds[field], nv); + if (err_kind) *err_kind = DB_ERR_OOM; + return -1; + } + + /* unique shadow-check: run with the NEW value before anything durable or + indexed moves, exactly row_apply_field_slot's promise */ + uint64_t old_vm = r->slots[field]; + r->slots[field] = nv_vm; + for (uint32_t x = 0; x < t->index_cnt; x++) { + db_index *ix = &t->indexes[x]; + if (!(ix->flags & 1u)) continue; + int touches = 0; + for (uint32_t i = 0; i < ix->col_cnt; i++) + if (ix->cols[i] == field) touches = 1; + if (!touches) continue; + db_ibucket *b = idx_bucket(ix, idx_hash(c, ix, r), 0); + if (!b) continue; + for (uint32_t i = 0; i < b->len; i++) { + if (b->ids[i] == id) continue; + const char *obmsg = ""; + db_row *other = wo_row_borrow(db, class_id, b->ids[i], &obmsg); + int clash = other && idx_cols_equal(c, ix, r, other); + wo_row_release(db, class_id, other); + if (clash) { + r->slots[field] = old_vm; /* untouched, promised */ + wo_row_release(db, class_id, r); + wo_drop_kind(db->rt, c->kinds[field], nv_vm); + db_val_free(c->kinds[field], nv); + if (err_kind) *err_kind = DB_ERR_UNIQUE; + *msg = "unique index violation"; + return -1; + } + } + } + r->slots[field] = old_vm; /* restored: still the OLD row until committed */ + + wo_wal *w = (wo_wal *)db->rt->wal; + uint64_t new_off = wo_wal_next_offset(w); + if (wo_wal_append_delta(w, db, class_id, id, field, back_off, nv) != 0) { + wo_row_release(db, class_id, r); + wo_drop_kind(db->rt, c->kinds[field], nv_vm); + db_val_free(c->kinds[field], nv); + if (err_kind) *err_kind = DB_ERR_OOM; + *msg = "out of memory appending delta"; + return -1; + } + if (wo_wal_commit(w) != 0) { + wo_row_release(db, class_id, r); + wo_drop_kind(db->rt, c->kinds[field], nv_vm); + db_val_free(c->kinds[field], nv); + *msg = "wal commit failed"; + return -1; + } + db_val_free(c->kinds[field], nv); /* durable now; the engine copy served the log */ + + /* commit: the borrowed row is the OLD row — out of every index under the + OLD value, then in again under the NEW one — then the id map moves */ + idx_remove_row(db, t, r); + r->slots[field] = nv_vm; + (void)idx_add_row(db, t, r); /* cannot violate uniqueness: the shadow + check above already cleared it */ + wo_row_set_offset(db, class_id, id, new_off); + wo_drop_kind(db->rt, c->kinds[field], old_vm); + wo_row_release(db, class_id, r); + if (err_kind) *err_kind = DB_ERR_NONE; + return 0; +} + int wo_row_update_field_slot(wo_db *db, uint32_t class_id, uint64_t id, uint32_t field, uint64_t slot, const char **msg, int *err_kind) { if (err_kind) *err_kind = DB_ERR_MISC; - /* databasev2 2 (5d): same reason as wo_row_update_field — a keys-resident - * row has no slab slot to mutate, and writing into the borrow's scratch - * would discard the update silently. Read-modify-APPEND is the shape that - * works, and it is not built yet. */ - if (wo_table_is_keys_resident(db, class_id)) { - *msg = "update on a `resident: keys` table is not implemented"; - return -1; - } /* bounds first: the RPC requester validated cid/field to encode at all, so these are defensive; the slot's kind is unknowable on a class violation and the value leaks rather than dies by the wrong kind */ @@ -1123,6 +1237,10 @@ int wo_row_update_field_slot(wo_db *db, uint32_t class_id, uint64_t id, uint32_t *msg = "no such field"; return -1; } + /* databasev2 3 (keys-resident delta updates): a keys-resident row has no + * slab slot to mutate — row_apply_field_keys does read-modify-APPEND. */ + if (wo_table_is_keys_resident(db, class_id)) + return row_apply_field_keys(db, class_id, id, field, slot, msg, err_kind); db_row *r = wo_row_ptr(db, class_id, id); if (!r) { db_val_free(c->kinds[field], slot); diff --git a/runtime/test/test_wal.c b/runtime/test/test_wal.c index 0d44584..015c0e1 100644 --- a/runtime/test/test_wal.c +++ b/runtime/test/test_wal.c @@ -856,6 +856,131 @@ static void test_fold_refuses_forward_pointing_delta(void) { wo_rt_destroy(&rt); } +/* Task 3 (keys-resident delta updates): the plain case, through the real + * API — wo_row_update_field, not a hand-rolled append+commit+set_offset like + * the fold tests above. Before this task it refused outright with "update on + * a `resident: keys` table is not implemented". */ +static void test_keys_resident_update_field(void) { + char path[128]; + snprintf(path, sizeof path, "%s/keysupd.wal", g_dir); + wo_rt rt; + T_EQ(wo_rt_init(&rt, 1 << 20, KEYS_CLASSES, 1), 0); + wo_db db; + T_EQ(wo_db_init(&db, KEYS_CLASSES, 1, 0, 1), 0); + wo_wal w; + T_EQ(wo_wal_open(&w, path, 1 << 16), 0); + db.rt = &rt; rt.wal = &w; rt.db = &db; + const char *msg = ""; + + wo_str *s = wo_str_new(&rt, "hello", 5); + uint64_t vals[2] = {111, (uint64_t)(uintptr_t)s}; + uint64_t id = wo_row_insert(&db, 0, vals, &msg, NULL); + T_CHECK(id != 0); + uint64_t off = wo_wal_next_offset(&w); + T_EQ(wo_wal_append_insert(&w, &db, 0, id), 0); + T_EQ(wo_wal_commit(&w), 0); + T_EQ(wo_row_drop_payload(&db, 0, id, off), 0); + + int ek = 0; + T_EQ(wo_row_update_field(&db, 0, id, 0, 999, &msg, &ek), 0); + T_EQ(ek, DB_ERR_NONE); + + db_row *r = wo_row_borrow(&db, 0, id, &msg); + T_CHECK(r != NULL); + T_CHECK(r->slots[0] == 999); + wo_str *back = (wo_str *)(uintptr_t)r->slots[1]; + T_CHECK(back != NULL && back->len == 5 && memcmp(back->data, "hello", 5) == 0); + wo_row_release(&db, 0, r); + + /* the scratch must be free again — a release that skipped clearing + scratch_busy would wedge this second borrow */ + db_row *again = wo_row_borrow(&db, 0, id, &msg); + T_CHECK(again != NULL && again->slots[0] == 999); + wo_row_release(&db, 0, again); + + wo_wal_close(&w); + wo_db_destroy(&db); + wo_rt_destroy(&rt); +} + +/* class 0: Row { n: scalar @index(non-unique), label: Text } — a keys-resident + * table with a secondary index on the scalar column, dedicated to the test + * below. The design deliberately allows a delta to change an indexed column + * (a catalogue indexes exactly the columns that change, like `price`), so + * this is the realistic case. */ +static const uint8_t keys_idx_kinds[] = {WO_K_SCALAR, WO_K_TEXT}; +static const uint32_t keys_idx_meta[] = {0 /*non-unique*/, 1, 0 /*col: n*/}; +static const wo_classdesc KEYS_IDX_CLASSES[] = { + {.name = 0, .flags = WO_CLASSF_RESIDENT_KEYS, .field_cnt = 2, .kinds = keys_idx_kinds, + .idx_cnt = 1, .idx_meta = keys_idx_meta}, +}; + +/* Task 3, the test that matters: updating an INDEXED column on a + * keys-resident row must move the row in the index too, not just in the + * log — queried through wo_idx_probe, the row is found by its NEW value and + * gone from its OLD one. */ +static void test_keys_resident_update_indexed(void) { + char path[128]; + snprintf(path, sizeof path, "%s/keysidx.wal", g_dir); + wo_rt rt; + T_EQ(wo_rt_init(&rt, 1 << 20, KEYS_IDX_CLASSES, 1), 0); + wo_db db; + T_EQ(wo_db_init(&db, KEYS_IDX_CLASSES, 1, 0, 1), 0); + wo_wal w; + T_EQ(wo_wal_open(&w, path, 1 << 16), 0); + db.rt = &rt; rt.wal = &w; rt.db = &db; + const char *msg = ""; + + wo_str *sa = wo_str_new(&rt, "a", 1); + wo_str *sb = wo_str_new(&rt, "b", 1); + uint64_t va[2] = {100, (uint64_t)(uintptr_t)sa}; + uint64_t vb[2] = {200, (uint64_t)(uintptr_t)sb}; + uint64_t a = wo_row_insert(&db, 0, va, &msg, NULL); + uint64_t b = wo_row_insert(&db, 0, vb, &msg, NULL); + T_CHECK(a != 0 && b != 0); + uint64_t off_a = wo_wal_next_offset(&w); + T_EQ(wo_wal_append_insert(&w, &db, 0, a), 0); + T_EQ(wo_wal_commit(&w), 0); + T_EQ(wo_row_drop_payload(&db, 0, a, off_a), 0); + uint64_t off_b = wo_wal_next_offset(&w); + T_EQ(wo_wal_append_insert(&w, &db, 0, b), 0); + T_EQ(wo_wal_commit(&w), 0); + T_EQ(wo_row_drop_payload(&db, 0, b, off_b), 0); + + /* before the update: probing 100 finds a */ + uint64_t *ids; + uint32_t cnt; + T_EQ(wo_idx_probe(&db, 0, 0, 100, NULL, 0, &ids, &cnt), 1); + T_CHECK(cnt == 1 && ids[0] == a); + free(ids); + + int ek = 0; + T_EQ(wo_row_update_field(&db, 0, a, 0, 150, &msg, &ek), 0); + T_EQ(ek, DB_ERR_NONE); + + /* found by the NEW value */ + T_EQ(wo_idx_probe(&db, 0, 0, 150, NULL, 0, &ids, &cnt), 1); + T_CHECK(cnt == 1 && ids[0] == a); + free(ids); + + /* gone from the OLD one */ + T_EQ(wo_idx_probe(&db, 0, 0, 100, NULL, 0, &ids, &cnt), 1); + T_CHECK(cnt == 0 && ids == NULL); + + /* b, untouched, still finds by its own value */ + T_EQ(wo_idx_probe(&db, 0, 0, 200, NULL, 0, &ids, &cnt), 1); + T_CHECK(cnt == 1 && ids[0] == b); + free(ids); + + db_row *r = wo_row_borrow(&db, 0, a, &msg); + T_CHECK(r != NULL && r->slots[0] == 150); + wo_row_release(&db, 0, r); + + wo_wal_close(&w); + wo_db_destroy(&db); + wo_rt_destroy(&rt); +} + /* databasev2 2 (5c): boot. A keys-resident store must come back from replay * with its rows readable FROM THE LOG — the map rebuilt to offsets, not slabs. * This is the half the round-trip test cannot cover: it runs in a fresh db, @@ -1504,6 +1629,8 @@ int main(void) { test_delta_fold_same_field_newest_wins(); test_fold_refuses_self_pointing_delta(); test_fold_refuses_forward_pointing_delta(); + test_keys_resident_update_field(); + test_keys_resident_update_indexed(); test_keys_resident_replay(); test_keys_resident_survives_compaction(); test_keys_resident_delete();