fix(polars): exclude nulls from nunique and approx_nunique - #12038
Open
mkzung wants to merge 1 commit into
Open
Conversation
mkzung
force-pushed
the
fix/polars-nunique-null
branch
from
August 7, 2026 11:20
8984441 to
d844635
Compare
The polars backend mapped CountDistinct and ApproxCountDistinct straight to
polars' n_unique and approx_n_unique, both of which count null as a distinct
value. The SQL backends emit COUNT(DISTINCT ...), which ignores nulls, so the
answer differed by backend:
t = ibis.memtable({"x": ["a", "b", "a", None]}, schema={"x": "string"})
t.x.nunique() # duckdb 2, polars 3
t.x.approx_nunique() # duckdb 2, polars 3
An all-null column shows the same split: 0 on SQL backends, 1 on polars.
The shared backend tests already treat nulls as excluded, using
.dropna().nunique() and .nunique() as the expected values, so polars was the
odd one out. Give both ops their own translation that drops nulls first,
following the CountDistinctStar handler next to them.
Adds a test for the exact case. approx_nunique is left untested there because
its result is approximate and already xfailed for exactness on some backends.
mkzung
force-pushed
the
fix/polars-nunique-null
branch
from
August 14, 2026 17:16
d844635 to
4a0f5da
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
nuniqueandapprox_nuniquecount nulls on the polars backend but not on the SQL backends:An all-null column shows the same split: 0 on the SQL backends, 1 on polars.
The polars compiler maps
CountDistinctto polars'n_uniqueandApproxCountDistincttoapprox_n_unique, and both treat null as a distinct value. The SQL backends compile toCOUNT(DISTINCT ...), which ignores nulls. The shared backend tests already assume nulls are excluded, using.dropna().nunique()and.nunique()as the expected values, so polars is the odd one out. The existing tests do not catch it because the column they aggregate has no nulls.This gives both ops their own translation that drops nulls before counting, following the
CountDistinctStarhandler next to them, and adds a test with a null and an all-null column.approx_nuniqueis not in that test because its result is approximate and already xfailed for exactness on some backends.polars is the only backend with its own compiler, so nothing else changes here: the rest go through the SQL compiler and already ignore nulls.
I checked that duckdb and polars agree afterwards for null, all-null and no-null columns, for
where=(including a null in the predicate), forgroup_by(...).agg(...), and across int, float, bool, string, date, timestamp, decimal, array and struct columns. Those are the two backends I can run locally, so if another one needs a mark here, happy to add it