Skip to content

fix: exec_round should round half away from zero#7400

Open
arianfarid wants to merge 2 commits into
tursodatabase:mainfrom
arianfarid:fix/round-half-away-from-zero
Open

fix: exec_round should round half away from zero#7400
arianfarid wants to merge 2 commits into
tursodatabase:mainfrom
arianfarid:fix/round-half-away-from-zero

Conversation

@arianfarid

@arianfarid arianfarid commented Jun 8, 2026

Copy link
Copy Markdown

Description

ROUND(x, d) was using Rust's format!("{:.N}", f) to perform rounding, which applies round-half-to-even (banker's rounding). SQLite uses round-half-away-from-zero.

This resulted in inconsistent rounding results for values halfway between precision.

This fix replaces the string format roundtrip by scaling the input by 10^precision, and using mul_add to recover the rounding errors in scaling.

This also eliminates the per call heap allocation of Rust's format! approach.

Motivation and context

Fixes #5748

Description of AI Usage

Claude Code was used to verify correctness, explore non-naive rounding solutions, and locally establish baseline and post-fix benchmarks.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Fossier: Approved by Maintainer

Approved by @pthorpe92.

@codspeed-hq

codspeed-hq Bot commented Jun 8, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×2.2

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
✅ 634 untouched benchmarks
⏩ 105 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation round_with_precision 6 µs 1.3 µs ×4.6
Simulation round_high_precision 6.7 µs 1.7 µs ×3.9
Simulation glob_question_multiple 1.7 µs 1.5 µs +15.9%
Simulation glob_question_single 1.7 µs 1.5 µs +15.82%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing arianfarid:fix/round-half-away-from-zero (3f9802a) with main (c70a032)

Open in CodSpeed

Footnotes

  1. 105 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@arianfarid

Copy link
Copy Markdown
Author

Hello,
Flagging something I found while testing this PR further.

The arithmetic approach in this PR diverges from SQLite on values like 2.675:

This PR: round(2.675, 2) → 2.68
SQLite: round(2.675, 2) → 2.67

The cause: 2.675 * 100 evaluates to exactly 267.5 in f64, so .round() takes it to 268 → 2.68. SQLite avoids this by rounding on the decimal representation via printf rather than scaling the binary float. It sees the stored value ≈2.6749999 and rounds to 2.67.

Happy to look into a fix that matches SQLite's behavior more closely.

@arianfarid

Copy link
Copy Markdown
Author

Update: the second commit (3f9802a) resolves the 2.675 divergence I flagged above.

The original (f * multiplier).round() / multiplier was wrong. The multiply rounds before round() runs. 2.675 * 100 is exactly 267.5 in f64, so it rounded up to 2.68 (but should return 2.67 in SQLite).

The fix recovers the exact error of that multiply with mul_add. This way, the rounding decision is made against the true product rather than its rounded approximation. This matches SQLite on the halfway cases (2.25, 2.675, 0.15, etc.). Note: values past ~15 significant digits could still differ from SQLite, but this is past reliable precision for f64.

@PThorpe92

Copy link
Copy Markdown
Collaborator

/fossier approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ROUND() uses banker's rounding instead of round-half-away-from-zero

2 participants