fix: floor `//` on DuckDB for negative operands and floats by ReguiguiMohamed · Pull Request #3994 · narwhals-dev/narwhals

ReguiguiMohamed · 2026-09-27T09:55:00Z

Description

DuckDB's // truncates ints toward zero and on floats it's true division, so -7 // 2 gave -3 and 7.0 // 2.0 gave 3.5. Now the int quotient steps down by one when the remainder is nonzero and the signs differ, and float quotients get floored like Polars.

expr	before	after
`-7 // 2`	-3	-4
`-2**63 // 3`	-3074457345618258602	-3074457345618258603
`7.0 // 2.0`	3.5	3.0
`-7.0 // 2.0`	-3.5	-4.0

Unsigned types stay unsigned and nothing overflows (10 // (2**63-1) is 0).

Floats follow Polars and PyArrow, so 1.0 // 0.1 is 10. pandas follows Python and gives 9, so that case is xfailed for pandas. The large-int test xfails ibis, sqlframe and pyspark since they floor through float64. I left that for a separate issue.

What type of PR is this? (check all applicable)

Related issues

Closes floor division truncates toward zero on duckdb (and 0//0 gives 0 on pandas) while every other backend floors #3984

AI assistance

No AI tools were used for this PR.
AI tools were used.

Checklist

Code follows style guide (ruff)
Tests added
Documented the changes
If this is your first PR to narwhals, attach a screenshot of pytest passing locally (not CI):

Screenshot

Summary by CodeRabbit

Bug Fixes
- Corrected DuckDB floor-division results for negative operands and inexact quotients, including floating-point, large-integer, and unsigned-value cases. This brings results in line with expected floor-division behavior.

DuckDB's `//` truncates integers toward zero and is true division on floats. Step the truncated integer quotient down by one when the remainder is nonzero and the signs differ, and floor the float quotient, as Polars does. Integer types are kept and no intermediate value can overflow. Before: -7 // 2 = -3, -2**63 // 3 = -3074457345618258602, 7.0 // 2.0 = 3.5. After: -4, -3074457345618258603, 3.0.

FBruzzesi

Thanks for the PR @ReguiguiMohamed, and for the clear before/after table!

The approach is right: correcting the truncated quotient keeps everything in integer arithmetic.

A few things before we can merge:

test_floordiv_large_integers fails on old polars: before 0.20.7 computes integer // through float64, and the minimum-versions and pretty-old-versions CI jobs pin 0.20.4. I checked: 0.20.4 fails both assertions and 0.20.8 passes. Suggestion inline.
Nothing tests the typeof check: if you delete it and apply the integer rule to floats, every test still passes. Only inputs like -1.0 // 0.1 tell the two rules apart: the quotient is exactly -10.0 but the remainder is nonzero, so the integer rule gives -11. That case also shows the xfail should be keyed on where Python and Polars actually disagree, not on the divisor. Suggestion inline.
Put the two rules in their own CASE arms: DuckDB resolves typeof when it binds the query and drops CASE arms it can prove dead. In the flat int_fix | (q < t) there is no arm to drop, so integer rows also evaluate the float comparison. On 20M rows (sum(a // b), best of 5, DuckDB 1.5.6), integers go from 32.8 ms to 24.1 ms. Floats stay the same (15.3 vs 14.8 ms) and both versions give identical results. Suggestion inline, together with more descriptive names.

Happy to push these to your branch if that's easier, just let me know.

Checked, no change needed

Unsigned integers: the signs never differ, so the step-down never runs and cannot underflow.
Nulls, inf and NaN pass through unchanged.
DECIMAL: DuckDB already returns DOUBLE for DECIMAL //, so it takes the float arm.

Not for this PR

Pre-existing, for follow-up issues:

DuckDB raises Overflow in division for -2**63 // -1. Polars returns -2**63.
x // 0.0 returns null on DuckDB because of the zero guard in _sql/expr.py, which every SQL backend shares. Polars returns inf, -inf or NaN.
Ibis, SQLFrame and PySpark lose precision past 2**53, as you noted in the description.

…check

coderabbitai · 2026-10-12T15:31:15Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 37855567-d6ae-4a34-a6a8-7d5e95a1ea89

📥 Commits

Reviewing files that changed from the base of the PR and between 6169bd1 and 3a1fad6.

📒 Files selected for processing: 2

src/narwhals/_duckdb/utils.py
tests/expr_and_series/arithmetic_test.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

DuckDB floor division now adjusts negative inexact quotients to match floor semantics. Arithmetic tests cover negative operands, null propagation, large and unsigned integers, and floating-point values.

Changes: DuckDB floor division

Layer / File(s)	Summary
Floor division implementation `src/narwhals/_duckdb/utils.py`	A new helper adjusts negative inexact quotients. The DuckDB binary-operation mapping uses it for `floordiv`.
Arithmetic validation `tests/expr_and_series/arithmetic_test.py`	Tests cover negative operands, null propagation, large and unsigned integers, and floating-point cases. DuckDB-specific floor-division xfails are removed.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: fbruzzesi

Merge Risk: ⚪ Minimal · up to 3a1fa

The supplied evidence identifies no concrete regression in the DuckDB floor-division correction or its arithmetic coverage. The change is ready to merge, subject to normal checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to 3a1fa

The change affects 2 systems.

Changed systems: src, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — src (service) was modified; 1 changed file maps to changed impact.
observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

observed — Modified behavior in src/narwhals/_duckdb/utils.py: The TYPE_CHECKING imports now include Callable for the binary-operation mapping annotation.
observed — Modified behavior in src/narwhals/_duckdb/utils.py: Adds _floordiv, which builds a truncating quotient expression and adjusts it downward for negative inexact division: floats are detected by quotient type and compared with their truncation; other types use a nonzero-remainder and opposite-sign check.
observed — Modified behavior in src/narwhals/_duckdb/utils.py: _BINARY_OPS now has a Mapping[str, Callable[[Expression, Expression], Expression]] annotation and routes "floordiv" to _floordiv rather than operator.floordiv. The "and" mapping and function dispatch remain in place.
observed — Modified behavior in tests/expr_and_series/arithmetic_test.py: Added POLARS_VERSION to the imports from tests.utils.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the primary change: fixing DuckDB floor division for negative operands and floating-point values.
Description check		The description explains the bug, expected behavior, implementation scope, affected backends, tests, linked issue, AI assistance, and checklist status. It also includes the requested test screenshot.
Linked Issues check		Issue `#3984` requires DuckDB floor division for negative operands. `_floordiv` replaces the direct DuckDB `//` mapping, adjusts integer results when the remainder is nonzero and operand signs differ, a…
Out of Scope Changes check		The changes stay within floor-division behavior. The type annotation supports the new operation mapping. The tests and removed DuckDB xfails verify the same behavior across operand forms and numeric t…

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

ReguiguiMohamed · 2026-10-12T15:57:23Z

Thanks for the careful review, and for checking all those cases. I applied all four suggestions in 6169bd1, fixing a small typo in the last one ("cudf"m). I also confirmed that -1.0 // 0.1 is the only case that fails on DuckDB once the typeof arm is removed. The arithmetic tests, ruff and mypy pass locally.

I'm happy to open the follow-up issues for the overflow on -2**63 // -1 and the x // 0.0 null if you'd like them.

FBruzzesi

Thanks for applying everything @ReguiguiMohamed! This looks good to me 🙌🏼

I'm happy to open the follow-up issues for the overflow on -2**63 // -1 and the x // 0.0 null if you'd like them.

Yes please to the two follow-up issues. They're out of scope here: neither is about rounding direction, and both affect more than DuckDB. Here is what I see on main, in case it helps:

Float floor division by zero: every backend except Polars returns null for [1.0, -1.0, 0.0] // 0.0: pandas, pandas[nullable], pandas[pyarrow], pyarrow, Dask, DuckDB, SQLFrame and Ibis. Polars computes floor(a / b) with IEEE division, so it returns inf, -inf and NaN, and only returns null for integers. Since the other backends agree with each other, I'd open this as a question rather than a bug: should float // 0.0 follow Polars, or stay null? division_by_zero_test.py only covers the integer column, so nothing pins either behaviour yet.
-2**63 // -1: Polars, pandas, pandas[nullable] and Dask wrap around to -2**63. pyarrow, pandas[pyarrow] and DuckDB raise an overflow error, Ibis raises a conversion error, and SQLFrame returns 9.223372036854776e+18. The SQLFrame result is the most urgent of these, because it's silently wrong rather than an error.

Please include the repro and the per-backend results in each issue.

ReguiguiMohamed · 2026-10-13T22:31:46Z

Thanks again @FBruzzesi for the review! Opened #4070 and #4071 for the two follow-ups.

ReguiguiMohamed mentioned this pull request Sep 27, 2026

floor division truncates toward zero on duckdb (and 0//0 gives 0 on pandas) while every other backend floors #3984

Closed

FBruzzesi added fix duckdb Issue is related to duckdb backend labels Oct 12, 2026

FBruzzesi requested changes Oct 12, 2026

View reviewed changes

Comment thread src/narwhals/_duckdb/utils.py Outdated

Comment thread src/narwhals/_duckdb/utils.py Outdated

Comment thread tests/expr_and_series/arithmetic_test.py Outdated

Comment thread tests/expr_and_series/arithmetic_test.py Outdated

Apply review: CASE arm per dtype, typed _BINARY_OPS, test the typeof …

6169bd1

…check

Merge branch 'main' into fix/duckdb-floordiv

3a1fad6

FBruzzesi approved these changes Oct 13, 2026

View reviewed changes

This was referenced Oct 13, 2026

[Enh](discussion): float // 0.0 returns null on every backend except Polars, which one is intended? #4070

Open

[Bug]: -2**63 // -1 inconsistent across backends, SQLFrame silently returns a float #4071

Open

FBruzzesi merged commit 030b984 into narwhals-dev:main Oct 14, 2026
42 checks passed

Navigation Menu

Repository navigation

fix: floor `//` on DuckDB for negative operands and floats - #3994

fix: floor `//` on DuckDB for negative operands and floats#3994
FBruzzesi merged 3 commits into
narwhals-dev:mainfrom
ReguiguiMohamed:fix/duckdb-floordiv

ReguiguiMohamed commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

FBruzzesi left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

ReguiguiMohamed commented Oct 12, 2026

Uh oh!

FBruzzesi left a comment

Uh oh!

ReguiguiMohamed commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ReguiguiMohamed commented Sep 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What type of PR is this? (check all applicable)

Related issues

AI assistance

Checklist

Screenshot

Summary by CodeRabbit

Uh oh!

FBruzzesi left a comment

Choose a reason for hiding this comment

Checked, no change needed

Not for this PR

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ReguiguiMohamed commented Oct 12, 2026

Uh oh!

FBruzzesi left a comment

Choose a reason for hiding this comment

Uh oh!

ReguiguiMohamed commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ReguiguiMohamed commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading