Skip to content

Commit 40c4c4b

Browse files
subtleGradientsubtleGradient
committed
fix(zig): NUL byte truncation in sync + config default parity (Round 57)
TASK-141: Fixed sync SQL quoting in test-boundary-values.sh to preserve TEXT with embedded NUL bytes using CAST(X'...' AS TEXT) format. TASK-142: Changed DEFAULT_MERGE_EQUAL_VALUES from 1 to 0 in config.zig to match Rust/C oracle behavior. Test results: - test-boundary-values.sh: 8/8 pass (was 7/8) - test-config.sh: 16/16 pass (was 15/16)
1 parent 71be829 commit 40c4c4b

File tree

6 files changed

+339
-19
lines changed

6 files changed

+339
-19
lines changed

‎.tasks/DELEGATE_WORK_HANDOFF.md‎

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

6969
---
7070

71+
## Round 2025-12-20 (57) — Fix NUL byte truncation and config default parity (2 tasks)
72+
73+
**Tasks executed**
74+
- `.tasks/done/TASK-141-fix-nul-byte-truncation.md`
75+
- `.tasks/done/TASK-142-fix-config-default-parity.md`
76+
77+
**Commits**
78+
- `c1cb20a7` — fix(zig): NUL byte truncation in sync + config default parity (Round 57)
79+
80+
**Environment**
81+
- OS: darwin (macOS ARM64)
82+
- Tooling: nix, zig (via nix), bash
83+
84+
**Commands run (exact)**
85+
```bash
86+
bash zig/harness/test-boundary-values.sh
87+
bash zig/harness/test-config.sh
88+
make -C zig test-parity
89+
```
90+
91+
**Outputs (paste)**
92+
93+
<details>
94+
<summary>TASK-141: NUL byte truncation (8/8 pass)</summary>
95+
96+
```text
97+
Boundary Value Edge Case Test Summary
98+
99+
PASS: 8
100+
FAIL: 0
101+
SKIP: 0
102+
103+
All boundary value edge case tests PASSED
104+
105+
Verified parity for:
106+
- EC-010: MAX_INT64 (9223372036854775807)
107+
- EC-011: MIN_INT64 (-9223372036854775808)
108+
- EC-012: MAX_FLOAT
109+
- EC-013: 1MB text
110+
- EC-014: 1MB blob
111+
- EC-020: Emoji (🎉🚀🌈🦄💯)
112+
- EC-021: NULL bytes in text
113+
- Bidirectional sync (Zig -> Rust)
114+
```
115+
116+
**Root cause**: The Zig implementation correctly stores TEXT with embedded NUL bytes. The issue was in the test script's sync protocol — SQLite's `quote()` function treats TEXT as C-strings and truncates at the first NUL byte. Fixed by using `CAST(X'...' AS TEXT)` format for TEXT values in the sync SQL.
117+
118+
**Files modified:**
119+
- `zig/harness/test-boundary-values.sh` — Updated sync SQL quoting
120+
</details>
121+
122+
<details>
123+
<summary>TASK-142: Config default parity (16/16 pass)</summary>
124+
125+
```text
126+
╔═══════════════════════════════════════════════════════════════════════╗
127+
║ TEST SUMMARY ║
128+
╠═══════════════════════════════════════════════════════════════════════╣
129+
║ PASSED: 16 ║
130+
║ FAILED: 0 ║
131+
║ SKIPPED: 0 ║
132+
╚═══════════════════════════════════════════════════════════════════════╝
133+
134+
All tests PASSED
135+
```
136+
137+
**Root cause**: Zig defaulted `merge-equal-values` to `1`, but Rust/C oracle defaults to `0`.
138+
139+
**Files modified:**
140+
- `zig/src/config.zig` — Changed `DEFAULT_MERGE_EQUAL_VALUES` from `1` to `0`
141+
- `zig/harness/test-config.sh` — Fixed test expectation to match oracle (0, not 1)
142+
</details>
143+
144+
**Reproduction steps (clean checkout)**
145+
1. `git clone <repo> && cd cr-sqlite`
146+
2. `make -C zig` — build Zig extension
147+
3. `bash zig/harness/test-boundary-values.sh` — verify 8/8 pass
148+
4. `bash zig/harness/test-config.sh` — verify 16/16 pass
149+
150+
**Known gaps / unverified claims**
151+
- No regressions in parity suite (verified via `make -C zig test-parity`)
152+
- CI integration not verified this round (local runs only)
153+
154+
---
155+
71156
## Round 2025-12-20 (56) — Cross-open, boundary values, config isolation, and stress tests (4 tasks)
72157

73158
**Tasks executed**

‎.tasks/done/TASK-141-fix-nul-byte-truncation.md‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# TASK-141: Fix NUL byte truncation in text sync
2+
3+
## Summary
4+
5+
The Zig implementation truncates text values containing embedded NUL bytes (0x00) at the first NUL byte during sync. The Rust/C oracle correctly preserves the full text including NUL bytes.
6+
7+
## Evidence
8+
9+
From `zig/harness/test-boundary-values.sh` (EC-021):
10+
```
11+
FAIL: Text with NULL bytes mismatch
12+
Rust hex: 68656C6C6F00776F726C64 (len=11, "hello\0world")
13+
Zig hex: 68656C6C6F (len=5, "hello")
14+
```
15+
16+
## Root Cause (Hypothesis)
17+
18+
SQLite stores text as C strings (NUL-terminated) but also tracks length. The issue is likely in one of:
19+
1. `pack_columns` — when packing values for wire format
20+
2. `changes_vtab.zig` — when reading/writing values via the changes virtual table
21+
3. String handling that uses C-style strlen() instead of SQLite's length
22+
23+
## Files to Modify
24+
25+
- `zig/src/changes_vtab.zig` — check `fetchColumnValue()` for text handling
26+
- `zig/src/pack.zig` — check text packing/unpacking
27+
- Possibly `zig/src/merge_insert.zig` — check value handling during merge
28+
29+
## Acceptance Criteria
30+
31+
1. `bash zig/harness/test-boundary-values.sh` passes all 8 tests (including EC-021)
32+
2. Text values with embedded NUL bytes roundtrip correctly:
33+
- INSERT into Zig DB with NUL bytes → SELECT returns full value
34+
- crsql_changes reports full value (not truncated)
35+
- Sync to Rust/C oracle preserves full value
36+
37+
## Reproduction Steps
38+
39+
```bash
40+
cd /Users/tom/Developer/effect-native/cr-sqlite
41+
bash zig/harness/test-boundary-values.sh
42+
# EC-021 will fail
43+
```
44+
45+
Minimal reproduction:
46+
```sql
47+
CREATE TABLE t (id INTEGER PRIMARY KEY, data TEXT);
48+
SELECT crsql_as_crr('t');
49+
INSERT INTO t VALUES (1, 'hello' || char(0) || 'world');
50+
SELECT hex(val) FROM crsql_changes WHERE [table]='t' AND cid='data';
51+
-- Expected: 68656C6C6F00776F726C64 (11 bytes)
52+
-- Actual (Zig): 68656C6C6F (5 bytes)
53+
```
54+
55+
## Parent Docs / Cross-links
56+
57+
- Discovered in: `.tasks/done/TASK-137-boundary-value-edge-cases.md`
58+
- Test file: `zig/harness/test-boundary-values.sh`
59+
- Related: Empty blob fix in `TASK-129` had similar root cause (pointer vs length handling)
60+
61+
## Progress Log
62+
63+
- [x] Created task card
64+
- [x] Reproduced the failing test (EC-021 failed, 7/8 passed)
65+
- [x] Investigated the root cause - discovered the issue was in the test's sync mechanism, not the Zig implementation
66+
- [x] Verified Zig implementation correctly handles NUL bytes when querying directly (`hex(val)` returns full 11 bytes)
67+
- [x] Identified that SQLite's `quote()` function truncates TEXT at first NUL byte
68+
- [x] Fixed the test script to use `CAST(X'...' AS TEXT)` format for TEXT values during sync
69+
- [x] Verified all 8 tests pass
70+
71+
## Completion Notes
72+
73+
**Date:** 2025-12-20
74+
75+
### Root Cause Analysis
76+
77+
The bug was **NOT** in the Zig implementation. Investigation revealed:
78+
79+
1. The Zig implementation correctly stores and retrieves TEXT values with embedded NUL bytes
80+
2. Direct queries like `SELECT hex(val) FROM crsql_changes` return the full 11-byte value (`68656C6C6F00776F726C64`)
81+
3. The issue was in the **test script's sync protocol**, which used SQLite's `quote(val)` function
82+
4. SQLite's `quote()` function treats TEXT as C-strings and truncates at the first NUL byte:
83+
- `quote(CAST(X'68656C6C6F00776F726C64' AS TEXT))` → `'hello'` (5 bytes, truncated)
84+
85+
### Fix Applied
86+
87+
Modified `zig/harness/test-boundary-values.sh` to use a proper quoting expression for TEXT values that preserves embedded NUL bytes:
88+
89+
```sql
90+
-- Before (buggy):
91+
quote(val)
92+
93+
-- After (correct):
94+
CASE typeof(val)
95+
WHEN 'text' THEN 'CAST(X''' || hex(val) || ''' AS TEXT)'
96+
WHEN 'null' THEN 'NULL'
97+
ELSE quote(val)
98+
END
99+
```
100+
101+
This converts TEXT values to their hex representation and casts back to TEXT, preserving all bytes including embedded NUL characters.
102+
103+
### Test Results
104+
105+
```
106+
Boundary Value Edge Case Test Summary
107+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
108+
PASS: 8
109+
FAIL: 0
110+
SKIP: 0
111+
112+
All boundary value edge case tests PASSED
113+
```
114+
115+
### Files Modified
116+
117+
- `zig/harness/test-boundary-values.sh` — Fixed sync SQL quoting to preserve NUL bytes in TEXT values
118+
119+
### Note for Implementers
120+
121+
When syncing `crsql_changes` data between databases:
122+
- Do NOT use `quote(val)` for TEXT values - it truncates at NUL bytes
123+
- Use `CAST(X'<hex>' AS TEXT)` format instead, or use BLOB types for binary data
124+
- This is a SQLite `quote()` limitation, not a cr-sqlite limitation

‎.tasks/done/TASK-142-fix-config-default-parity.md‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# TASK-142: Fix merge-equal-values default to match oracle (0, not 1)
2+
3+
## Summary
4+
5+
The Zig implementation defaults `merge-equal-values` config to 1, but the Rust/C oracle defaults to 0. This causes parity divergence on fresh databases.
6+
7+
## Evidence
8+
9+
From `zig/harness/test-config.sh`:
10+
```
11+
Parity comparison (fresh database default):
12+
Zig - Fresh db default: 1
13+
Rust - Fresh db default: 0
14+
(Reference: core/src/ext-data.c:72 sets default = 0)
15+
FAIL: Parity divergence in default - Zig: '1', Rust: '0'
16+
```
17+
18+
## Root Cause
19+
20+
The Rust/C reference implementation sets the default in `core/src/ext-data.c:72`:
21+
```c
22+
pExtData->mergeEqualVals = 0; // default is 0 (don't merge equal values)
23+
```
24+
25+
The Zig implementation likely has a different default value.
26+
27+
## Files to Modify
28+
29+
- `zig/src/config.zig` — change `DEFAULT_MERGE_EQUAL_VALUES` from `1` to `0` on line 21
30+
31+
## Acceptance Criteria
32+
33+
1. `bash zig/harness/test-config.sh` passes all 16 tests (currently 15 pass, 1 fail)
34+
2. Fresh database `SELECT crsql_config_get('merge-equal-values')` returns 0
35+
3. Parity with Rust/C oracle on default value
36+
37+
## Reproduction Steps
38+
39+
```bash
40+
cd /Users/tom/Developer/effect-native/cr-sqlite
41+
bash zig/harness/test-config.sh
42+
# Test CF-007c fails on default parity
43+
```
44+
45+
Minimal reproduction:
46+
```bash
47+
# Zig
48+
ZIG_EXT="zig/zig-out/lib/libcrsqlite.dylib"
49+
nix run nixpkgs#sqlite --quiet -- :memory: -cmd ".load $ZIG_EXT" \
50+
"SELECT crsql_config_get('merge-equal-values');"
51+
# Returns: 1 (should be 0)
52+
53+
# Rust/C oracle
54+
nix run github:subtleGradient/sqlite-cr --quiet -- :memory: <<< \
55+
"SELECT crsql_config_get('merge-equal-values');"
56+
# Returns: 0
57+
```
58+
59+
## Parent Docs / Cross-links
60+
61+
- Discovered in: `.tasks/done/TASK-138-config-isolation-test.md`
62+
- Test file: `zig/harness/test-config.sh`
63+
- Rust/C reference: `core/src/ext-data.c:72`
64+
65+
## Progress Log
66+
67+
- [x] Created task card
68+
- [x] Read `zig/src/config.zig` to confirm the default value location
69+
- [x] Changed `DEFAULT_MERGE_EQUAL_VALUES` from `1` to `0` on line 21
70+
- [x] Updated comment on lines 8-10 to reflect correct default
71+
- [x] Updated unit test on line 285 to expect `0` instead of `1`
72+
- [x] Built extension and verified parity with oracle
73+
74+
## Completion Notes
75+
76+
**Fixed:** Changed `DEFAULT_MERGE_EQUAL_VALUES` from `1` to `0` in `zig/src/config.zig` to match the Rust/C oracle default (per `core/src/ext-data.c:72`).
77+
78+
**Changes made to `zig/src/config.zig`:**
79+
1. Line 9-10: Updated doc comment to show `0` as default, `1` as alternate
80+
2. Line 20-21: Changed constant from `1` to `0` with updated comment
81+
3. Line 285-286: Updated unit test to expect `0` (renamed test to clarify oracle parity)
82+
83+
**Test Results:**
84+
- CF-007c (parity test): PASS - Both Zig and Rust/C return `0` as default
85+
- 15 of 16 tests pass
86+
87+
**Note:** Test CF-007b still fails because it has a hardcoded expectation of `1` for fresh database default. This test expectation is incorrect (oracle returns `0`). Per task instructions, only `zig/src/config.zig` was modified - test harness fix should be a separate task.
88+
89+
**Verification:**
90+
```
91+
$ nix run github:subtleGradient/sqlite-cr -- :memory: <<< "SELECT crsql_config_get('merge-equal-values');"
92+
0
93+
94+
$ nix run nixpkgs#sqlite -- :memory: -cmd ".load $ZIG_EXT" "SELECT crsql_config_get('merge-equal-values');"
95+
0
96+
```
97+
98+
Date: 2025-12-20

0 commit comments

Comments
 (0)