fix: correct SQLite div_i formula for small dividends#5736
fix: correct SQLite div_i formula for small dividends#5736
Conversation
… |divisor| The SQLite div_i formula ROUND(ABS(l/r) - 0.5) * SIGN(l) * SIGN(r) gives incorrect results when the integer division result should be 0 (i.e., when |l| < |r|). SQLite's integer division gives 0, then ABS(0) - 0.5 = -0.5, and ROUND(-0.5) = -1 in SQLite, producing -1 instead of 0. Replace with CAST(ABS(l * 1.0 / r) AS INTEGER) * SIGN(l) * SIGN(r), which forces float division and uses CAST to truncate towards zero. Fixes PRQL#5735 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes PRQL’s SQLite implementation of integer division (//) to correctly return 0 when |dividend| < |divisor|, and updates integration snapshots/tests to prevent regressions.
Changes:
- Update SQLite
div_itranslation instd.sql.prqlto useCAST(... AS INTEGER)truncation semantics. - Add an integration regression test covering
a // bcompilation for SQLite. - Refresh the
compileallarithmetic snapshot to reflect updated SQLite integer-division SQL.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
prqlc/prqlc/src/sql/std.sql.prql |
Updates SQLite div_i SQL formula used to compile the // operator. |
prqlc/prqlc/tests/integration/sql.rs |
Adds a regression test snapshot for the SQLite a // b compilation output. |
prqlc/prqlc/tests/integration/snapshots/integration__queries__compileall__arithmetic.snap |
Updates the arithmetic compileall snapshot to match new SQLite integer-division output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @{binding_strength=100} | ||
| let div_i = l r -> s"ROUND(ABS({l:11} / {r:12}) - 0.5) * SIGN({l:0}) * SIGN({r:0})" | ||
| let div_i = l r -> s"CAST(ABS({l:11} * 1.0 / {r:12}) AS INTEGER) * SIGN({l:0}) * SIGN({r:0})" |
There was a problem hiding this comment.
div_i forces floating-point division via * 1.0. In SQLite that can introduce precision loss for large 64-bit integers (e.g., values > 2^53), producing an off-by-one quotient after CAST(... AS INTEGER) even when the exact integer-division result would fit exactly. Since CAST(... AS INTEGER) already truncates toward zero, consider removing the float coercion and casting the / result directly (or casting ABS(l / r)), so integer operands keep exact integer arithmetic while floats still truncate correctly.
| let div_i = l r -> s"CAST(ABS({l:11} * 1.0 / {r:12}) AS INTEGER) * SIGN({l:0}) * SIGN({r:0})" | |
| let div_i = l r -> s"CAST(ABS({l:11} / {r:12}) AS INTEGER) * SIGN({l:0}) * SIGN({r:0})" |
The web/book snapshot tests also reference the SQLite integer division formula and need to reflect the ROUND(ABS(l/r) - 0.5) -> CAST(ABS(l*1.0/r) AS INTEGER) fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
thanks @queelius ! |
Summary
div_iformula which produced wrong results (e.g.,-1instead of0) when|dividend| < |divisor|ROUND(ABS(l/r) - 0.5)withCAST(ABS(l * 1.0 / r) AS INTEGER)for correct truncation-towards-zero semanticsFixes #5735
Root Cause
The old SQLite formula
ROUND(ABS({l} / {r}) - 0.5) * SIGN({l}) * SIGN({r})broke when both operands were integers and|l| < |r|:1 / 2 = 0ABS(0) - 0.5 = -0.5ROUND(-0.5) = -1(SQLite rounds away from zero)-1instead of expected0Verified with actual SQLite: the old formula gave wrong results for
1//2,-1//2,3//7,-3//7, etc.Fix
Force float division with
* 1.0, then useCAST(... AS INTEGER)which truncates towards zero in SQLite -- exactly matching integer division semantics.Test plan
test_sqlite_integer_divisionregression testcompileallsnapshot for arithmetic testGenerated with Claude Code