Skip to content

nth_value and approx_percentile_cont silently ignore DISTINCT #26497

Open

Description

@jayzhan211

Describe the bug

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

CREATE TABLE t(x DOUBLE) AS VALUES (1.0), (1.0), (1.0), (2.0), (3.0);

SELECT nth_value(x, 2 ORDER BY x), nth_value(DISTINCT x, 2 ORDER BY x) FROM t;
-- 1 1

SELECT approx_percentile_cont(x, 0.5), approx_percentile_cont(DISTINCT x, 0.5) FROM t;
-- 1 1

SELECT 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.

Activity

  1. CuteChuanChuan commented on Oct 19, 2026

    @CuteChuanChuan
    Contributor

    take

  2. PreetiToppo commented on Oct 20, 2026

    @PreetiToppo

    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!

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions