Skip to content

Commit 30877c5

Browse files
fix(zig): use lazy materialize semantics for ALTER ADD COLUMN (TASK-101)
- Removed backfillNewColumns() call from crsqlCommitAlterFunc - Clock entries are now only created when column is explicitly written - Matches Rust/C oracle behavior exactly - test-alter-parity.sh: 19/19 pass
1 parent 2b73c43 commit 30877c5

File tree

5 files changed

+184
-66
lines changed

5 files changed

+184
-66
lines changed

.tasks/DELEGATE_WORK_HANDOFF.md

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,105 @@ Artifacts:
6868

6969
---
7070

71+
## Round 2025-12-20 (50) — Implement lazy ALTER ADD COLUMN semantics (1 task)
72+
73+
**Tasks executed**
74+
- `.tasks/done/TASK-101-impl-alter-add-column-no-backfill.md`
75+
76+
**Commits**
77+
- (pending commit)
78+
79+
**Environment**
80+
- OS: darwin (macOS ARM64)
81+
- Tooling: nix, zig (via nix), bash
82+
83+
**Commands run (exact)**
84+
```bash
85+
bash zig/harness/test-alter-parity.sh
86+
```
87+
88+
**Outputs (paste)**
89+
90+
<details>
91+
<summary>TASK-101: test-alter-parity.sh (19/19 pass)</summary>
92+
93+
```text
94+
╔═══════════════════════════════════════════════════════════════════════╗
95+
║ ALTER TABLE Parity Test (Zig vs Rust/C Oracle) ║
96+
╚═══════════════════════════════════════════════════════════════════════╝
97+
98+
Rust/C: using sqlite-cr (nix run github:subtleGradient/sqlite-cr)
99+
Zig extension: /Users/tom/Developer/effect-native/cr-sqlite/zig/zig-out/lib/libcrsqlite.dylib
100+
101+
Test 1: ADD COLUMN (nullable)
102+
PASS: Pre-alter state - clock states match
103+
PASS: Post-ADD COLUMN (nullable) - clock states match
104+
PASS: After UPDATE on new column - clock states match
105+
106+
Test 2: ADD COLUMN with DEFAULT
107+
PASS: Pre-alter state - clock states match
108+
PASS: Post-ADD COLUMN with DEFAULT - clock states match
109+
110+
Test 3: DROP COLUMN
111+
PASS: Pre-DROP state - clock states match
112+
PASS: Post-DROP COLUMN - clock states match
113+
PASS: Dropped column clock entries removed
114+
115+
Test 4: ADD INDEX / DROP INDEX
116+
PASS: Pre-INDEX state - clock states match
117+
PASS: Post-ADD INDEX - clock states match
118+
PASS: Post-DROP INDEX - clock states match
119+
120+
Test 5: ALTER on empty table
121+
PASS: Empty table ALTER handled (both have 0 clock entries)
122+
123+
Test 6: ALTER on table with 1000+ rows
124+
Rust clock entries: 1000
125+
Zig clock entries: 1000
126+
PASS: 1000-row ALTER clock count matches
127+
128+
Test 7: Multiple ALTERs in sequence
129+
PASS: Sequential ALTERs - dropped column removed from both
130+
131+
Test 8: ADD COLUMN then immediately UPDATE
132+
PASS: UPDATE on new column worked in both
133+
Clock entries for 'newcol': Rust=1, Zig=1
134+
PASS: Both have clock entries for new column after UPDATE
135+
136+
Test 9: Existing clock history preserved
137+
PASS: Rust/C preserved existing clock entries
138+
PASS: Zig preserved existing clock entries
139+
140+
Test 10: New column backfill behavior
141+
Backfill clock entries: Rust=0, Zig=0
142+
INFO: Rust does NOT backfill clock entries for new columns
143+
PASS: Backfill behavior documented
144+
145+
╔═══════════════════════════════════════════════════════════════════════╗
146+
║ ALTER PARITY TEST SUMMARY ║
147+
╠═══════════════════════════════════════════════════════════════════════╣
148+
║ PASSED: 19 ║
149+
║ FAILED: 0 ║
150+
╚═══════════════════════════════════════════════════════════════════════╝
151+
152+
All ALTER parity tests PASSED
153+
```
154+
155+
**Change made**: Removed `backfillNewColumns()` call from `crsqlCommitAlterFunc` in `zig/src/schema_alter.zig`.
156+
157+
Zig now uses **LAZY MATERIALIZE** semantics for ALTER ADD COLUMN, matching the Rust/C oracle behavior exactly.
158+
</details>
159+
160+
**Reproduction steps (clean checkout)**
161+
1. `git clone <repo> && cd cr-sqlite`
162+
2. `bash zig/harness/test-alter-parity.sh` — verify 19/19 pass
163+
164+
**Known gaps / unverified claims**
165+
- No coverage captured
166+
- CI integration not verified this round (local runs only)
167+
168+
---
169+
71170
## Round 2025-12-20 (49) — Fix merge pk/rowid confusion + ALTER semantics decision (3 tasks)
72171

73172
**Tasks executed**

.tasks/backlog/TASK-101-impl-alter-add-column-no-backfill.md

Lines changed: 0 additions & 55 deletions
This file was deleted.
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# TASK-101: Align Zig ALTER ADD COLUMN clocks with oracle (no eager backfill)
2+
3+
## Status
4+
- [x] Planned
5+
- [x] Assigned
6+
- [x] In Progress
7+
- [ ] Blocked (reason: ...)
8+
- [x] Complete
9+
10+
## Priority
11+
high
12+
13+
## Assigned To
14+
(completed)
15+
16+
## Parent Docs / Cross-links
17+
- Oracle parity test: `zig/harness/test-alter-parity.sh`
18+
- Zig alter implementation: `zig/src/schema_alter.zig`
19+
- Rust/C alter implementation (reference): `core/rs/core/src/alter.rs`
20+
- sqlite-cr wrapper: `nix run github:subtleGradient/sqlite-cr -- ...`
21+
- Origin task: `.tasks/active/TASK-094-alter-table-history-preservation.md`
22+
- Decision task: `.tasks/backlog/TASK-100-decide-alter-new-column-clock-semantics.md`
23+
24+
## Description
25+
After TASK-094, Zig diverged from the oracle on `ALTER TABLE ... ADD COLUMN`:
26+
27+
- Zig eagerly backfilled `__crsql_clock` entries for the new column.
28+
- Oracle (sqlite-cr / Rust/C) does not create clock rows for new columns until the column is explicitly written.
29+
30+
This task implements the **oracle-matching behavior** in Zig (LAZY MATERIALIZE semantics), and the parity test is now fully green.
31+
32+
## Files to Modify
33+
- [x] `zig/src/schema_alter.zig`
34+
- [x] `zig/harness/test-alter-parity.sh` (no changes needed - already correct)
35+
- [ ] `zig/harness/test-parity.sh` (no changes needed)
36+
37+
## Acceptance Criteria
38+
- [x] `zig/harness/test-alter-parity.sh` passes with **0 failures**.
39+
- [x] After `ADD COLUMN` (nullable) with existing rows:
40+
- [x] Zig does **not** create `__crsql_clock` rows for the new column.
41+
- [x] Existing column clock rows are unchanged.
42+
- [x] After `ADD COLUMN ... DEFAULT ...` with existing rows:
43+
- [x] Zig does **not** create `__crsql_clock` rows for the new column.
44+
- [x] Later explicit `UPDATE` to the new column creates a clock row with `col_version = 1`.
45+
- [x] After sequential alters (add column then add column then drop column):
46+
- [x] Zig clock column set matches oracle when columns are never written.
47+
- [x] Large-table case (1000+ rows) does not backfill clock rows.
48+
49+
## Progress Log
50+
### 2025-12-20
51+
- Evidence from `zig/harness/test-alter-parity.sh` shows Zig eager-backfill causes:
52+
- extra clock rows after `ADD COLUMN`
53+
- different `col_version` after first UPDATE to new column
54+
55+
### 2025-12-20 (completion)
56+
- Removed `backfillNewColumns()` call from `crsqlCommitAlterFunc` in `zig/src/schema_alter.zig`
57+
- Updated docstring to document LAZY MATERIALIZE semantics
58+
- Rebuilt Zig extension
59+
- All 19 parity tests now pass with 0 failures
60+
61+
## Completion Notes
62+
### 2025-12-20
63+
64+
**Change made**: Removed the call to `backfillNewColumns()` from `crsqlCommitAlterFunc` in `zig/src/schema_alter.zig`.
65+
66+
**Rationale**: Zig now uses LAZY MATERIALIZE semantics for ALTER ADD COLUMN, matching the Rust/C oracle behavior. Clock entries are only created when a column is explicitly written (via UPDATE or INSERT), not eagerly when the column is added.
67+
68+
**Test results**: `bash zig/harness/test-alter-parity.sh` - 19 PASSED, 0 FAILED
69+
70+
**Files changed**:
71+
- `zig/src/schema_alter.zig` - Removed backfill call, updated docstring

research/zig-cr/92-gap-backlog.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# 92-gap-backlog
22

3-
> Last updated: 2025-12-20 (Round 49 — TASK-119, TASK-100 complete)
3+
> Last updated: 2025-12-20 (Round 50 — TASK-101 complete)
44
55
## Status
66

@@ -101,7 +101,9 @@ Goal: invalidate the hypothesis that "Zig is done" by expanding cross-implementa
101101
- [x] **TASK-100** — Decide ADD COLUMN clock semantics ✓ `.tasks/done/TASK-100-decide-alter-new-column-clock-semantics.md`
102102
- **Decision: LAZY MATERIALIZE** — Zig should NOT backfill clock entries on ADD COLUMN
103103
- Rationale: Clock entries represent writes, not schema changes; matches oracle; O(0) sync payload
104-
- [ ] **TASK-101** — Implement lazy semantics in Zig → `.tasks/backlog/TASK-101-impl-alter-add-column-no-backfill.md`
104+
- [x] **TASK-101** — Implement lazy semantics in Zig ✓ `.tasks/done/TASK-101-impl-alter-add-column-no-backfill.md`
105+
- Removed `backfillNewColumns()` call from `crsqlCommitAlterFunc`
106+
- Test: `zig/harness/test-alter-parity.sh` = 19/19 pass ✓
105107
- [x] **TASK-102** — Fix/replace local oracle dylib (ALTER) ✓ `.tasks/done/TASK-102-fix-oracle-crsqlite-dylib-alter.md`
106108

107109
### Realistic Sync Tests

zig/src/schema_alter.zig

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,12 @@ fn crsqlBeginAlterFunc(
8484
/// 1. Clean up clock entries for removed columns
8585
/// 2. Re-read table schema via PRAGMA table_info
8686
/// 3. Recreate triggers with new column list
87-
/// 4. Backfill clock entries for new columns with col_version=1
88-
/// 5. Release the savepoint
89-
/// 6. Return NULL on success, error on failure
87+
/// 4. Release the savepoint
88+
/// 5. Return NULL on success, error on failure
89+
///
90+
/// NOTE: Zig uses LAZY MATERIALIZE semantics for ADD COLUMN (matches oracle).
91+
/// Clock entries are NOT created eagerly; they are only created when the
92+
/// column is explicitly written (UPDATE/INSERT).
9093
fn crsqlCommitAlterFunc(
9194
pCtx: ?*api.sqlite3_context,
9295
argc: c_int,
@@ -147,12 +150,10 @@ fn crsqlCommitAlterFunc(
147150
return;
148151
};
149152

150-
// Step 3: Backfill clock entries for new columns
151-
backfillNewColumns(db, table_name_ptr) catch {
152-
api.result_error(pCtx, "crsql_commit_alter: failed to backfill new columns", -1);
153-
_ = api.exec(db, "ROLLBACK TO alter_crr", null, null, null);
154-
return;
155-
};
153+
// NOTE: Zig uses LAZY MATERIALIZE semantics for ADD COLUMN (matches oracle).
154+
// Clock entries are NOT created eagerly on ADD COLUMN.
155+
// They are only created when the column is explicitly written (UPDATE/INSERT).
156+
// This matches Rust/C oracle behavior.
156157

157158
// Release savepoint on success
158159
if (api.exec(db, "RELEASE alter_crr", null, null, null) != api.SQLITE_OK) {

0 commit comments

Comments
 (0)