Skip to content

Commit d729882

Browse files
authored
Merge 'fix: adjust jump_if_null branch offset as incorrectly failed NULL in check constraints in integrity checks' from Pedro Muniz
Caught by differential fuzzer `PRAGMA integrity_check` incorrectly flagged rows with NULL values as CHECK constraint violations. Per the SQL standard, NULL does not violate CHECK constraints — SQLite correctly allows NULLs and reports `ok` on integrity check, but Turso's integrity check reported false positives like: ``` CHECK constraint failed in t1 ``` This was the root cause of 3 fuzzer-reported "data corruption" failures (seeds: `4713306089706541904`, `13879125450750994315`, `16798752381237836570`). The data itself was correct — only the integrity check diagnosis was wrong. ### Reproducer ```sql CREATE TABLE t1 (id INTEGER PRIMARY KEY, val REAL CHECK (val > 0)); INSERT INTO t1 (id, val) VALUES (1, NULL); -- NULL passes CHECK (correct) PRAGMA integrity_check; -- SQLite: ok -- Turso (before fix): CHECK constraint failed in t1 -- Turso (after fix): ok ``` ### Root Cause In `core/translate/expr.rs`, `emit_binary_condition_insn` controls the `jump_if_null` flag on comparison instructions (`Gt`, `Lt`, `Eq`, etc.). The flag was only set when `jump_if_condition_is_true == false` (the common WHERE clause path). When `jump_if_condition_is_true == true` (as used by integrity_check), NULL comparisons fell through to the error path instead of jumping to the "check ok" label. The integrity check emits CHECK validation with `jump_if_condition_is_true: true` and `jump_target_when_null: check_ok`, but the comparison instruction had no `jump_if_null` flag, so `NULL > 0` evaluated to NULL, didn't jump, and fell through to report a false violation. ### Fix Changed the `jump_if_null` flag logic to be driven by whether `jump_target_when_null` matches the jump target, rather than being hardcoded per direction: - `jump_if_condition_is_true == true`: set `jump_if_null` when `null_target == true_target` - `jump_if_condition_is_true == false`: set `jump_if_null` when `null_target == false_target` This is backward-compatible — all existing callers with `jump_if_condition_is_true == false` already have `null_target == false_target`. Claude did it all Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #5495
2 parents 03e4cf7 + 3ecc30d commit d729882

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

core/translate/expr.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3823,13 +3823,17 @@ fn emit_binary_condition_insn(
38233823
opposite_op
38243824
};
38253825

3826-
// Similarly, we "jump if NULL" only when we intend to jump if the condition is false.
3827-
let flags = if condition_metadata.jump_if_condition_is_true {
3828-
CmpInsFlags::default().with_affinity(affinity)
3829-
} else {
3830-
CmpInsFlags::default()
3831-
.with_affinity(affinity)
3832-
.jump_if_null()
3826+
// Set the "jump if NULL" flag when the NULL target matches the jump target.
3827+
// When jump_if_condition_is_true: we jump on true, so set jump_if_null when NULL should also jump (e.g. CHECK constraints in integrity_check).
3828+
// When !jump_if_condition_is_true: we jump on false, so set jump_if_null when NULL should also jump (standard SQL 3-valued logic).
3829+
let mut flags = CmpInsFlags::default().with_affinity(affinity);
3830+
if condition_metadata.jump_if_condition_is_true {
3831+
if condition_metadata.jump_target_when_null == condition_metadata.jump_target_when_true {
3832+
flags = flags.jump_if_null()
3833+
}
3834+
} else if condition_metadata.jump_target_when_null == condition_metadata.jump_target_when_false
3835+
{
3836+
flags = flags.jump_if_null()
38333837
};
38343838

38353839
let target_pc = if condition_metadata.jump_if_condition_is_true {

testing/runner/tests/check_constraint.sqltest

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,14 +1655,33 @@ expect {
16551655
# Bug fix: CHECK constraint type coercion must use column affinity (#5170)
16561656
# TEXT column comparing with integer literal should use TEXT affinity,
16571657
# meaning the integer is coerced to text for comparison.
1658-
test check_constraint_text_affinity_coercion_violation {
1659-
CREATE TABLE t(val TEXT CHECK(val > 5));
1660-
INSERT INTO t VALUES ('10');
1658+
# Bug fix: integrity_check must not flag NULL values as CHECK violations.
1659+
# Per SQL standard, NULL does not violate CHECK constraints.
1660+
@skip-if mvcc "MVCC integrity_check does not perform row-level CHECK validation"
1661+
test check_constraint_integrity_check_null_not_violation {
1662+
CREATE TABLE t(id INTEGER PRIMARY KEY, val REAL CHECK (val > 0));
1663+
INSERT INTO t (id, val) VALUES (1, 100.0);
1664+
INSERT INTO t (id, val) VALUES (2, NULL);
1665+
INSERT INTO t (id, val) VALUES (3, 200.0);
1666+
PRAGMA integrity_check;
16611667
}
1662-
expect error {
1663-
CHECK constraint failed: val > 5
1668+
expect {
1669+
ok
16641670
}
16651671

1672+
@skip-if mvcc "MVCC integrity_check does not perform row-level CHECK validation"
1673+
test check_constraint_integrity_check_detects_real_violation {
1674+
CREATE TABLE t(id INTEGER PRIMARY KEY, val INTEGER CHECK (val > 0));
1675+
PRAGMA ignore_check_constraints = ON;
1676+
INSERT INTO t VALUES (1, -5);
1677+
PRAGMA ignore_check_constraints = OFF;
1678+
PRAGMA integrity_check;
1679+
}
1680+
expect {
1681+
CHECK constraint failed in t
1682+
}
1683+
1684+
16661685
test check_constraint_text_affinity_coercion_pass {
16671686
CREATE TABLE t(val TEXT CHECK(val > 5));
16681687
-- '6' > '5' in text comparison is true

0 commit comments

Comments
 (0)