|
| 1 | +# TASK-186 — Decide schema mismatch behavior for unknown columns |
| 2 | + |
| 3 | +## Goal |
| 4 | +Decide whether Zig should error or ignore unknown columns during sync, then align implementations. |
| 5 | + |
| 6 | +## Status |
| 7 | +- State: done |
| 8 | +- Priority: MEDIUM (behavioral difference, not data corruption) |
| 9 | +- Discovered: Round 62 (TASK-173 test suite) |
| 10 | + |
| 11 | +## Problem |
| 12 | +When source has a column that destination doesn't have, the implementations differ: |
| 13 | +- **Zig**: Returns ERROR |
| 14 | +- **Rust/C**: Gracefully IGNORES the unknown column, syncs known columns |
| 15 | + |
| 16 | +**Test failure from `test-schema-mismatch.sh`:** |
| 17 | +``` |
| 18 | +Divergences found: |
| 19 | + - source_has_extra_column: Zig='ERROR' vs Rust='IGNORED' |
| 20 | +``` |
| 21 | + |
| 22 | +## Scenario |
| 23 | +1. Site A: table with columns (id, name, extra) |
| 24 | +2. Site B: table with columns (id, name) — no 'extra' column |
| 25 | +3. Site A: INSERT with extra='value' |
| 26 | +4. Sync A→B |
| 27 | +5. **Rust/C**: Row synced, 'extra' column data ignored, known columns applied |
| 28 | +6. **Zig**: ERROR returned, nothing synced |
| 29 | + |
| 30 | +## Analysis |
| 31 | +Both approaches have merit: |
| 32 | +- **Zig (strict)**: Catches schema drift early, prevents silent data loss |
| 33 | +- **Rust/C (lenient)**: Allows staggered migrations, more forgiving in production |
| 34 | + |
| 35 | +## Decision Made |
| 36 | +**Align with Rust/C (lenient behavior)** — ignore unknown columns during sync. |
| 37 | + |
| 38 | +Rationale: |
| 39 | +- Production systems with rolling upgrades need this |
| 40 | +- Strict behavior breaks staggered migration scenarios |
| 41 | +- Other columns in the same row should still be applied |
| 42 | +- Silent data "loss" is acceptable since the column doesn't exist locally anyway |
| 43 | + |
| 44 | +## Files Modified |
| 45 | +- `zig/src/merge_insert.zig` — added `columnExistsInTable()` helper |
| 46 | +- `zig/src/changes_vtab.zig` — added column existence check before merge |
| 47 | + |
| 48 | +## Acceptance Criteria |
| 49 | +1. [x] Decision documented |
| 50 | +2. [x] `bash zig/harness/test-schema-mismatch.sh` — All tests pass |
| 51 | +3. [x] `bash zig/harness/test-app-todo.sh` — Regression test passes |
| 52 | +4. [x] `bash zig/harness/test-parity.sh` — No new regressions |
| 53 | + |
| 54 | +## Parent Docs / Cross-links |
| 55 | +- Test: `zig/harness/test-schema-mismatch.sh` (Test 1: source_has_extra_column) |
| 56 | +- Triggering task: `.tasks/done/TASK-173-schema-mismatch.md` |
| 57 | + |
| 58 | +## Progress Log |
| 59 | +- 2025-12-22: Created from Round 62 divergence discovery. |
| 60 | +- 2025-12-25: Fixed by adding `columnExistsInTable()` check in `changes_vtab.zig`. |
| 61 | + - Root cause: When applying a change for a column that doesn't exist locally, |
| 62 | + `updateBaseTableColumn` or `insertOrUpdateColumn` would fail with SQLITE_ERROR |
| 63 | + because the generated SQL references a non-existent column. |
| 64 | + - Fix: Check if column exists before attempting to apply the change. |
| 65 | + If column doesn't exist, skip gracefully with SQLITE_OK (lenient mode). |
| 66 | + - Added check in two places: |
| 67 | + 1. Before inserting new rows (NoRows case) |
| 68 | + 2. Before updating existing rows (after sentinel handling) |
| 69 | + |
| 70 | +## Completion Notes |
| 71 | +- 2025-12-25: COMPLETED |
| 72 | +- Test results: `test-schema-mismatch.sh` — 12 passed, 0 failed |
| 73 | +- Regression check: `test-parity.sh` — 363 passed, 13 failed (pre-existing failures) |
| 74 | +- Key changes: |
| 75 | + - Added `pub fn columnExistsInTable()` in `merge_insert.zig:366-386` |
| 76 | + - Added column check at line ~1740 for new rows |
| 77 | + - Added column check at line ~1960 for existing rows |
0 commit comments