You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
nth_value and approx_percentile_cont silently ignore DISTINCT #26497
nth_value and approx_percentile_cont return the same result with DISTINCT as without it. This is the same problem #26471 reports for corr, covar_* and regr_*, which #26484 fixes. Both functions are tagged DistinctHandling::Unsupported, but their accumulators never read is_distinct.
To Reproduce
CREATETABLEt(x DOUBLE) ASVALUES (1.0), (1.0), (1.0), (2.0), (3.0);
SELECT nth_value(x, 2ORDER BY x), nth_value(DISTINCT x, 2ORDER BY x) FROM t;
-- 1 1SELECT approx_percentile_cont(x, 0.5), approx_percentile_cont(DISTINCT x, 0.5) FROM t;
-- 1 1SELECT approx_percentile_cont(0.5) WITHIN GROUP (ORDER BY x),
approx_percentile_cont(DISTINCT 0.5) WITHIN GROUP (ORDER BY x) FROM t;
-- 1 1
Expected behavior
The distinct values are {1, 2, 3}, so all three DISTINCT forms should return 2. If DataFusion does not support DISTINCT for these functions, the query should fail with an error instead of returning the non-distinct answer.
Additional context
#26484 adds a not_impl_err! check to each accumulator. Another option is a single check in AggregateExprBuilder::build: return not_impl_err! when is_distinct is set and the function reports DistinctHandling::Unsupported. Both aggregates and window functions go through that builder, and it runs after the logical optimizer, so SingleDistinctToGroupBy can still rewrite cases like corr(DISTINCT x, x). The default for distinct_handling is Sensitive, so user-defined functions that don't opt in are not affected. Future functions tagged Unsupported would then be covered automatically, and the error would come at planning time instead of execution time.
While @CuteChuanChuan is tackling the accumulators for #26497, I would love to implement the centralized architectural fix suggested in the context. I'll add a planning-time check inside AggregateExprBuilder::build that automatically returns a not_impl_err! if is_distinct is true and the function indicates DistinctHandling::Unsupported. I will open a PR for this refactor!
Activity
CuteChuanChuan commentedon Oct 19, 2026
take
PreetiToppo commentedon Oct 20, 2026
While @CuteChuanChuan is tackling the accumulators for #26497, I would love to implement the centralized architectural fix suggested in the context. I'll add a planning-time check inside
AggregateExprBuilder::buildthat automatically returns anot_impl_err!ifis_distinctis true and the function indicatesDistinctHandling::Unsupported. I will open a PR for this refactor!