Skip to content

Commit 5429f66

Browse files
triage: add follow-up tasks for TASK-123 regressions (TASK-125, TASK-126)
1 parent 8586645 commit 5429f66

File tree

2 files changed

+79
-0
lines changed

2 files changed

+79
-0
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# TASK-125: Fix schema_alter.zig pk→key column rename for clock table
2+
3+
## Status
4+
- [ ] Planned
5+
6+
## Priority
7+
high (blocks alter and merge tests)
8+
9+
## Assigned To
10+
(unassigned)
11+
12+
## Parent Docs / Cross-links
13+
- Caused by: `.tasks/done/TASK-123-fix-clock-table-schema-parity.md`
14+
- Test failures: `zig/harness/test-alter.sh` (Test 2b), `zig/harness/test-noops.sh` (Test 3)
15+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
16+
17+
## Description
18+
TASK-123 renamed the clock table column from `pk` to `key` in `as_crr.zig`, but missed updating `schema_alter.zig`. This causes SQL errors when:
19+
20+
1. **test-alter.sh Test 2b**: "table bar2__crsql_clock has no column named pk"
21+
2. **test-noops.sh Test 3**: "setWinnerClockCached failed" (SQL logic error)
22+
23+
The issue is that `schema_alter.zig` still references `"pk"` in clock table SQL at:
24+
- Line 264: CREATE TABLE clock definition
25+
- Line 270: PRIMARY KEY clause
26+
- Lines 468-473: INSERT INTO clock
27+
- Lines 578, 590: INSERT OR REPLACE INTO clock
28+
29+
## Files to Modify
30+
- `zig/src/schema_alter.zig` — update all clock table `"pk"` references to `"key"`
31+
32+
## Acceptance Criteria
33+
- [ ] All `"pk"` references in clock table SQL changed to `"key"`
34+
- [ ] Clock table creation includes `STRICT` and index (matching TASK-123)
35+
- [ ] `bash zig/harness/test-alter.sh` — 6/6 pass
36+
- [ ] `bash zig/harness/test-noops.sh` — 4/4 pass
37+
- [ ] No regressions in `make -C zig test-parity`
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# TASK-126: Fix merge resolution parity with oracle
2+
3+
## Status
4+
- [ ] Planned
5+
6+
## Priority
7+
medium
8+
9+
## Assigned To
10+
(unassigned)
11+
12+
## Parent Docs / Cross-links
13+
- Test: `zig/harness/test-oracle-parity.sh` (Test 3a, 3b)
14+
- Test: `zig/harness/test-parity.sh` (ValueWin test)
15+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
16+
17+
## Description
18+
The Zig implementation's merge resolution differs from the Rust/C oracle:
19+
20+
**Test 3a: Remote wins with higher col_version**
21+
- Zig: returns "local" (local value kept)
22+
- Rust/C: returns "remote_winner" (remote value applied)
23+
24+
**Test 3b: site_id tiebreaker (lower site_id wins on equal col_version)**
25+
- Zig: returns empty
26+
- Rust/C: returns "low_site"
27+
28+
**ValueWin test (test-parity.sh)**
29+
- Expected rows_impacted = 1
30+
- Got: empty (merge not applied)
31+
32+
This suggests the merge logic in `changes_vtab.zig` or `merge_insert.zig` is not correctly determining when remote wins.
33+
34+
## Files to Modify
35+
- `zig/src/changes_vtab.zig` — merge resolution logic
36+
- `zig/src/merge_insert.zig` — merge helpers
37+
38+
## Acceptance Criteria
39+
- [ ] `zig/harness/test-oracle-parity.sh` Test 3a passes
40+
- [ ] `zig/harness/test-oracle-parity.sh` Test 3b passes
41+
- [ ] `zig/harness/test-parity.sh` ValueWin test passes
42+
- [ ] No regressions in other parity tests

0 commit comments

Comments
 (0)