Skip to content

Commit a885618

Browse files
fix(zig): seq divergence + schema mismatch behavior (Round 76)
TASK-199: Fix seq value divergence (Zig=1 vs Rust=0) - Root cause: crsqlAfterInsertFunc called getNextSeq() unconditionally for maybeMarkReinserted, wasting seq=0 even when no sentinel existed - Fix: Modified getOrCreatePkKey() to return {key, existed} struct - Only bump seq for reinsert when existed=true (resurrection case) - Result: test-clock-internals.sh 27/27 pass, 0 seq divergences TASK-186: Align schema mismatch behavior with Rust/C - Decision: Lenient (ignore unknown columns during sync) - Added columnExistsInTable() helper in merge_insert.zig - Check column existence before applying changes in changes_vtab.zig - Result: test-schema-mismatch.sh 12/12 pass Test results: - test-parity.sh: 367 passed, 4 failed (pre-existing) - test-clock-internals.sh: 27/27 pass - test-app-todo.sh: 2 parity confirmed
1 parent b1ce108 commit a885618

File tree

8 files changed

+274
-83
lines changed

8 files changed

+274
-83
lines changed

.tasks/DELEGATE_WORK_HANDOFF.md

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

6969
---
7070

71+
## Round 2025-12-25 (76) — seq divergence + schema mismatch fixes (2 tasks)
72+
73+
**Tasks executed**
74+
- `.tasks/done/TASK-199-seq-value-divergence.md` (bug fix)
75+
- `.tasks/done/TASK-186-schema-mismatch-unknown-column-behavior.md` (behavior alignment)
76+
77+
**Commits**
78+
- `211de13e` — fix(zig): seq divergence + schema mismatch behavior (Round 76)
79+
80+
**Environment**
81+
- OS: darwin (macOS ARM64)
82+
- Tooling: nix, zig (via nix), bash
83+
84+
**Commands run (exact)**
85+
```bash
86+
make -C zig build
87+
bash zig/harness/test-clock-internals.sh
88+
bash zig/harness/test-schema-mismatch.sh
89+
bash zig/harness/test-app-todo.sh
90+
bash zig/harness/test-parity.sh
91+
```
92+
93+
**Outputs (paste)**
94+
95+
<details>
96+
<summary>TASK-199: seq divergence fix (27/27 PASS, 0 seq divergences)</summary>
97+
98+
**Root cause**: In `crsqlAfterInsertFunc`, Zig called `getNextSeq()` unconditionally for `maybeMarkReinserted()`, even when no sentinel row existed to update. This consumed `seq=0` without effect, causing the first column's seq to start at 1 instead of 0.
99+
100+
Rust/C only calls `bump_seq()` when `create_record_existed=true` (resurrection case).
101+
102+
**Fix**: Modified `getOrCreatePkKey()` to return a `GetOrCreateKeyResult` struct with both `key` and `existed` flag. Only call `getNextSeq()` for reinsert if `existed=true`.
103+
104+
**Test output**:
105+
```text
106+
Clock Table Internals Test Summary
107+
PASSED: 27
108+
FAILED: 0
109+
seq divergences: 0
110+
111+
Before fix: Zig seq=1, Rust seq=0
112+
After fix: Zig seq=0, Rust seq=0
113+
```
114+
115+
**Files modified**: `zig/src/local_writes/after_write.zig`
116+
</details>
117+
118+
<details>
119+
<summary>TASK-186: schema mismatch fix (12/12 PASS)</summary>
120+
121+
**Design decision**: Align with Rust/C (lenient behavior) — ignore unknown columns during sync.
122+
123+
**Root cause**: When applying a change for a column that doesn't exist locally, `updateBaseTableColumn()` or `insertOrUpdateColumn()` would fail with SQLITE_ERROR because the generated SQL references a non-existent column.
124+
125+
**Fix**: Added `columnExistsInTable()` helper in `merge_insert.zig`. Added column existence checks before applying changes in `changes_vtab.zig`. If column doesn't exist, skip gracefully.
126+
127+
**Test output**:
128+
```text
129+
Schema Mismatch Test Summary
130+
PASSED: 12
131+
FAILED: 0
132+
```
133+
134+
**Files modified**:
135+
- `zig/src/merge_insert.zig` — added `columnExistsInTable()`
136+
- `zig/src/changes_vtab.zig` — added column checks
137+
</details>
138+
139+
<details>
140+
<summary>Regression checks</summary>
141+
142+
```text
143+
test-app-todo.sh:
144+
Results: 2 parity confirmed, 0 failures, 0 divergences
145+
146+
test-parity.sh:
147+
PASSED: 367
148+
FAILED: 4 (pre-existing edge cases)
149+
SKIPPED: 22
150+
```
151+
</details>
152+
153+
**Reproduction steps (clean checkout)**
154+
1. `git clone <repo> && cd cr-sqlite`
155+
2. `make -C zig build`
156+
3. `bash zig/harness/test-clock-internals.sh` — verify 27/27 pass, 0 seq divergences
157+
4. `bash zig/harness/test-schema-mismatch.sh` — verify 12/12 pass
158+
5. `bash zig/harness/test-parity.sh` — verify 367 passed
159+
160+
**Known gaps / unverified claims**
161+
- Linux CI not verified (local darwin only)
162+
- No commits yet (pending)
163+
164+
---
165+
71166
## Round 2025-12-25 (75) — Composite PK sync + Hypothesis tests (2 tasks)
72167

73168
**Tasks executed**

.tasks/active/TASK-199-seq-value-divergence.md

Lines changed: 0 additions & 64 deletions
This file was deleted.

.tasks/active/TASK-186-schema-mismatch-unknown-column-behavior.md renamed to .tasks/done/TASK-186-schema-mismatch-unknown-column-behavior.md

File renamed without changes.
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# TASK-199 — seq Value Divergence (Zig=1, Rust=0)
2+
3+
## Goal
4+
Investigate and fix the `seq` column value divergence between Zig and Rust/C implementations.
5+
6+
## Status
7+
- State: **DONE**
8+
- Priority: MEDIUM (affects sync ordering in edge cases)
9+
- Discovered: 2025-12-23 (TASK-192 prior DB parity testing)
10+
- Fixed: 2025-12-25 (Round 76)
11+
12+
## Problem
13+
14+
When performing single operations, the `seq` column in `crsql_changes` differs:
15+
- **Rust/C**: `seq=0`
16+
- **Zig (before fix)**: `seq=1`
17+
18+
## Root Cause
19+
20+
In `crsqlAfterInsertFunc`, Zig called `getNextSeq()` unconditionally for `maybeMarkReinserted()`, even when no sentinel row existed to update. This consumed `seq=0` without effect, causing the first column's seq to start at 1.
21+
22+
**Rust/C behavior:**
23+
```rust
24+
// Only bump seq IF create_record_existed (resurrection case)
25+
} else if create_record_existed {
26+
let seq = bump_seq(ext_data);
27+
update_create_record(...);
28+
}
29+
// Then bump seq for each column
30+
for col in tbl_info.non_pks.iter() {
31+
let seq = bump_seq(ext_data); // First call returns 0
32+
...
33+
}
34+
```
35+
36+
**Zig (before fix):**
37+
```zig
38+
// Always called, even for fresh inserts
39+
const seq_reinsert = site_identity.getNextSeq(); // Returns 0, wasted
40+
_ = maybeMarkReinserted(...); // Does nothing if no sentinel
41+
42+
// Then for each column
43+
const seq2 = site_identity.getNextSeq(); // Returns 1, should be 0
44+
```
45+
46+
## Fix
47+
48+
Modified `getOrCreatePkKey()` to return a struct with both `key` and `existed` flag, matching Rust/C's approach. Then only call `maybeMarkReinserted()` when `existed=true`.
49+
50+
**Files modified:**
51+
- `zig/src/local_writes/after_write.zig`
52+
- Added `GetOrCreateKeyResult` struct
53+
- Modified `getOrCreatePkKey()` to return `{key, existed}`
54+
- Modified `crsqlAfterInsertFunc` to only bump seq for reinsert if existed
55+
- Updated `crsqlAfterUpdateFunc` and `crsqlAfterDeleteFunc` callers
56+
57+
## Verification
58+
59+
```bash
60+
# Before fix:
61+
Zig: t|^A ^A|x|1 # seq=1
62+
Rust: t|^A ^A|x|0 # seq=0
63+
64+
# After fix:
65+
Zig: t|^A ^A|x|0 # seq=0
66+
Rust: t|^A ^A|x|0 # seq=0
67+
```
68+
69+
Test results:
70+
- `test-clock-internals.sh`: 27 PASSED, 0 seq divergences
71+
- `test-app-todo.sh`: 2 parity confirmed
72+
- `test-parity.sh`: 367 PASSED
73+
74+
## Acceptance Criteria
75+
76+
1. [x] Document the intended semantics of `seq`
77+
2. [x] Determine if divergence affects sync correctness (yes, ordering)
78+
3. [x] Fix Zig to match Rust (done)
79+
80+
## Parent Docs / Cross-links
81+
82+
- Discovery: `.tasks/done/TASK-192-prior-db-oracle-parity.md`
83+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
84+
85+
## Progress Log
86+
- 2025-12-23: Created from TASK-192 findings.
87+
- 2025-12-25: Fixed in Round 76. Root cause: unconditional `getNextSeq()` for reinsert.
88+
89+
## Completion Notes
90+
- Date: 2025-12-25
91+
- Commit: 211de13e

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

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

3-
> Last updated: 2025-12-25 (Post-Round 75: Composite PK sync + Hypothesis tests)
3+
> Last updated: 2025-12-25 (Post-Round 76: seq divergence + schema mismatch fixes)
44
55
## Status
66

@@ -15,21 +15,22 @@
1515
- Resurrection parity: ✅ **25/25 PASSING** (Round 63)
1616
- Sentinel parity: ✅ **6/6 PASSING** (Round 63)
1717
- Multinode sync: ✅ **6/6 PASSING** (Round 63)
18-
- Schema mismatch: ⚠️ **11/12 PASSING** (1 divergence: unknown column behavior)
18+
- Schema mismatch: **12/12 PASSING** (Round 76 — unknown column now ignored)
1919
- Savepoint sync: ✅ **16/16 PASSING** (Round 67)
2020
- ATTACH CRR: ✅ **15/15 PASSING** (Round 66)
2121
- Site ID collision: ✅ **13/13 PASSING** (Round 66)
2222
- Trigger CRR: ✅ **31/31 PASSING** (Round 67)
2323
- VACUUM CRR: ✅ **17/17 PASSING** (Round 68)
2424
- Wide table: ✅ **13/13 PASSING** (Round 69)
2525
- PK UPDATE: ✅ **16/16 PASSING** (Round 74)
26+
- Clock internals: ✅ **27/27 PASSING, 0 seq divergences** (Round 76)
2627
- **App simulation (Todo)**: ✅ **2/2 PASSING** (Round 73)
2728
- **App simulation (Chat)**: ✅ **4/4 PASSING** (Round 73)
2829
- **App simulation (Inventory)**: ✅ **4/4 PASSING** (Round 75 — composite PK sync fixed)
2930
- **Stress test (60 iterations)**: ✅ **60/60 PASSING, 0 divergences** (Round 73)
3031
- **CL merge properties**: ✅ **18/18 PASSING** (Round 75)
3132
- **Sentinel properties**: ✅ **15/15 PASSING** (Round 75)
32-
- Parity suite: **357 passed, 13 failed (pre-existing), 22 skipped**
33+
- Parity suite: **367 passed, 4 failed (pre-existing edge cases), 22 skipped**
3334
- Test scripts: **67+ total**
3435
- Zig implementation: `zig/`
3536
- Canonical task queue: `.tasks/{backlog,active,done}/`
@@ -39,16 +40,14 @@
3940
### Active (0 tasks)
4041
No active tasks. Core sync functionality is complete and working.
4142

42-
### Backlog (2 tasks)
43+
### Backlog (1 task)
4344
| Task | Priority | Summary | Effort |
4445
|------|----------|---------|--------|
4546
| **TASK-207** | BLOCKED | Re-enable CI for release | Medium (blocked on release decision) |
46-
| **TASK-186** | MEDIUM | Schema mismatch: unknown column behavior | Design decision |
4747

48-
### Triage Inbox (4 items)
48+
### Triage Inbox (3 items)
4949
| Task | Priority | Summary | Disposition |
5050
|------|----------|---------|-------------|
51-
| **TASK-199** | MEDIUM | seq divergence (Zig=1, Rust=0) | Needs design decision |
5251
| **TASK-200** | LOW | Zig validation gaps (more permissive) | Nice to have |
5352
| **TASK-201** | LOW | Performance regression tests | Nice to have |
5453
| **TASK-203** | LOW | Empty blob PK encoding divergence | Edge case |
@@ -60,7 +59,18 @@ No active tasks. Core sync functionality is complete and working.
6059

6160
### Known Limitations
6261
- **crsql_changes SELECT perf**: ~2-7x slower on wide tables vs Rust/C (COUNT is fast, SELECT * is slow)
63-
- **seq divergence**: Zig starts seq at 1, Rust/C at 0 (doesn't affect sync correctness)
62+
63+
### Completed Round 76 (2025-12-25) — seq divergence + schema mismatch fixes
64+
- [x] **TASK-199**: Fix seq value divergence (Zig=1, Rust=0) ✓
65+
- Root cause: `crsqlAfterInsertFunc` called `getNextSeq()` unconditionally for `maybeMarkReinserted`, wasting seq=0
66+
- Fix: Modified `getOrCreatePkKey()` to return `{key, existed}` struct, only bump seq for reinsert if existed
67+
- Files: `zig/src/local_writes/after_write.zig`
68+
- Result: `test-clock-internals.sh` now 27/27 PASS with 0 seq divergences
69+
- [x] **TASK-186**: Fix schema mismatch unknown column behavior ✓
70+
- Decision: Align with Rust/C (lenient) — ignore unknown columns during sync
71+
- Fix: Added `columnExistsInTable()` helper, check before applying column changes
72+
- Files: `zig/src/merge_insert.zig`, `zig/src/changes_vtab.zig`
73+
- Result: `test-schema-mismatch.sh` now 12/12 PASS
6474

6575
### Completed Round 75 (2025-12-25) — Composite PK sync + Hypothesis tests
6676
- [x] **TASK-208**: Fix composite PK sync ✓

zig/src/changes_vtab.zig

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,15 @@ fn changesUpdate(
17311731
// No local row - INSERT new row
17321732
log.debug("changesUpdate: no local row, inserting new row", .{});
17331733

1734+
// Check if the column exists in the destination table.
1735+
// If not, skip this change gracefully (matches Rust/C lenient behavior).
1736+
// This handles staggered migrations where source has columns destination doesn't.
1737+
if (!merge_insert.columnExistsInTable(api_db, table_slice, cid_slice)) {
1738+
log.debug("changesUpdate: column '{s}' does not exist in table '{s}', skipping (lenient mode)", .{ cid_slice, table_slice });
1739+
pRowid.* = 0;
1740+
return vtab.SQLITE_OK;
1741+
}
1742+
17341743
// Get the value from argv[5] (column 3: val)
17351744
const insert_value = toApiValue(argv[5]);
17361745

@@ -1948,6 +1957,16 @@ fn changesUpdate(
19481957
return vtab.SQLITE_OK;
19491958
}
19501959

1960+
// Step 4a: Check if column exists in destination table.
1961+
// If not, skip this change gracefully (matches Rust/C lenient behavior).
1962+
// This handles staggered migrations where source has columns destination doesn't.
1963+
// Note: This check comes AFTER sentinel handling since sentinels (-1) always "exist".
1964+
if (!merge_insert.columnExistsInTable(api_db, table_slice, cid_slice)) {
1965+
log.debug("changesUpdate: column '{s}' does not exist in table '{s}', skipping (lenient mode)", .{ cid_slice, table_slice });
1966+
pRowid.* = 0;
1967+
return vtab.SQLITE_OK;
1968+
}
1969+
19511970
// Step 4b: Check if row needs resurrection
19521971
// A row needs resurrection if:
19531972
// - The incoming cl indicates live (odd) OR cl > local_cl

0 commit comments

Comments
 (0)