You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/21 21:28:03 UTC
(datafusion) branch gh-readonly-queue/main/pr-26484-e6a144f078c47ef3b22985e3436756ccd40ec524 created (now 61ec8455dc)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-26484-e6a144f078c47ef3b22985e3436756ccd40ec524
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 61ec8455dc fix: reject DISTINCT in corr, covar and regr_* instead of ignoring it (#26484)
This branch includes the following new commits:
new 61ec8455dc fix: reject DISTINCT in corr, covar and regr_* instead of ignoring it (#26484)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: reject DISTINCT in corr, covar and regr_* instead of ignoring it (#26484)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-26484-e6a144f078c47ef3b22985e3436756ccd40ec524
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 61ec8455dce23e1d128daae124b4cdb2ba44a791
Author: Alejandro Borrego <ab...@gmail.com>
AuthorDate: Wed Oct 21 20:27:38 2026 +0000
fix: reject DISTINCT in corr, covar and regr_* instead of ignoring it (#26484)
## Which issue does this PR close?
- Closes #26471.
## Rationale for this change
`corr`, `covar_pop`, `covar_samp` and the `regr_*` functions return the
same result with `DISTINCT` as without it, because their accumulators
never read `is_distinct`.
## What changes are included in this PR?
Their accumulators now return a not implemented error for `DISTINCT`, as
`stddev` and `approx_median` already do. `corr(DISTINCT x, x)` still
works, since `SingleDistinctToGroupBy` rewrites it before it reaches the
accumulator.
## What is the testing strategy for this PR?
New cases in `aggregate.slt`. The two `REGR_COUNT(DISTINCT ...)` results
in `eliminate_join_distinct.slt` were the non-distinct counts, so they
now expect the error.
## Are there any user-facing changes?
These queries now fail with an error instead of returning a wrong
result.
---------
Co-authored-by: Alejandro Borrego <21...@users.noreply.github.com>
---
datafusion/functions-aggregate/src/correlation.rs | 17 +++++++++------
datafusion/functions-aggregate/src/covariance.rs | 24 +++++++++++++--------
datafusion/functions-aggregate/src/regr.rs | 16 +++++++++-----
datafusion/sqllogictest/test_files/aggregate.slt | 25 ++++++++++++++++++++++
.../test_files/eliminate_join_distinct.slt | 17 ++++++---------
5 files changed, 67 insertions(+), 32 deletions(-)
diff --git a/datafusion/functions-aggregate/src/correlation.rs b/datafusion/functions-aggregate/src/correlation.rs
index e2877e63a1..f9110ccba7 100644
--- a/datafusion/functions-aggregate/src/correlation.rs
+++ b/datafusion/functions-aggregate/src/correlation.rs
@@ -38,7 +38,7 @@ use log::debug;
use crate::covariance::CovarianceAccumulator;
use crate::stddev::StddevAccumulator;
-use datafusion_common::{Result, ScalarValue};
+use datafusion_common::{Result, ScalarValue, not_impl_err};
use datafusion_expr::{
Accumulator, AggregateUDFImpl, Documentation, Signature, Volatility,
function::{AccumulatorArgs, StateFieldsArgs},
@@ -108,7 +108,10 @@ impl AggregateUDFImpl for Correlation {
Ok(DataType::Float64)
}
- fn accumulator(&self, _acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ if acc_args.is_distinct {
+ return not_impl_err!("CORR(DISTINCT) aggregations are not available");
+ }
Ok(Box::new(CorrelationAccumulator::try_new()?))
}
@@ -139,8 +142,8 @@ impl AggregateUDFImpl for Correlation {
self.doc()
}
- fn groups_accumulator_supported(&self, _args: AccumulatorArgs) -> bool {
- true
+ fn groups_accumulator_supported(&self, args: AccumulatorArgs) -> bool {
+ !args.is_distinct
}
fn create_groups_accumulator(
@@ -152,9 +155,9 @@ impl AggregateUDFImpl for Correlation {
}
fn distinct_handling(&self) -> DistinctHandling {
- // Duplicate-sensitive, but the accumulator does not read
- // `is_distinct` and today silently returns the non-distinct answer.
- // The tag records the intent; enforcement is a follow-up change.
+ // The accumulator rejects `DISTINCT` with `not_impl_err!`. It only
+ // works when the planner removes it first (`SingleDistinctToGroupBy`
+ // for a single distinct field, e.g. `corr(DISTINCT x, x)`).
DistinctHandling::Unsupported
}
}
diff --git a/datafusion/functions-aggregate/src/covariance.rs b/datafusion/functions-aggregate/src/covariance.rs
index 46abc82517..2477098728 100644
--- a/datafusion/functions-aggregate/src/covariance.rs
+++ b/datafusion/functions-aggregate/src/covariance.rs
@@ -20,7 +20,7 @@
use arrow::array::ArrayRef;
use arrow::datatypes::{DataType, Field, FieldRef};
use datafusion_common::cast::{as_float64_array, as_uint64_array};
-use datafusion_common::{Result, ScalarValue};
+use datafusion_common::{Result, ScalarValue, not_impl_err};
use datafusion_expr::DistinctHandling;
use datafusion_expr::{
Accumulator, AggregateUDFImpl, Documentation, Signature, Volatility,
@@ -118,7 +118,10 @@ impl AggregateUDFImpl for CovarianceSample {
.collect())
}
- fn accumulator(&self, _acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ if acc_args.is_distinct {
+ return not_impl_err!("COVAR_SAMP(DISTINCT) aggregations are not available");
+ }
Ok(Box::new(CovarianceAccumulator::try_new(StatsType::Sample)?))
}
@@ -131,9 +134,9 @@ impl AggregateUDFImpl for CovarianceSample {
}
fn distinct_handling(&self) -> DistinctHandling {
- // Duplicate-sensitive, but the accumulator does not read
- // `is_distinct` and today silently returns the non-distinct answer.
- // The tag records the intent; enforcement is a follow-up change.
+ // The accumulator rejects `DISTINCT` with `not_impl_err!`. It only
+ // works when the planner removes it first (`SingleDistinctToGroupBy`
+ // for a single distinct field, e.g. `covar_samp(DISTINCT x, x)`).
DistinctHandling::Unsupported
}
}
@@ -205,7 +208,10 @@ impl AggregateUDFImpl for CovariancePopulation {
.collect())
}
- fn accumulator(&self, _acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ if acc_args.is_distinct {
+ return not_impl_err!("COVAR_POP(DISTINCT) aggregations are not available");
+ }
Ok(Box::new(CovarianceAccumulator::try_new(
StatsType::Population,
)?))
@@ -216,9 +222,9 @@ impl AggregateUDFImpl for CovariancePopulation {
}
fn distinct_handling(&self) -> DistinctHandling {
- // Duplicate-sensitive, but the accumulator does not read
- // `is_distinct` and today silently returns the non-distinct answer.
- // The tag records the intent; enforcement is a follow-up change.
+ // The accumulator rejects `DISTINCT` with `not_impl_err!`. It only
+ // works when the planner removes it first (`SingleDistinctToGroupBy`
+ // for a single distinct field, e.g. `covar_pop(DISTINCT x, x)`).
DistinctHandling::Unsupported
}
}
diff --git a/datafusion/functions-aggregate/src/regr.rs b/datafusion/functions-aggregate/src/regr.rs
index 6d679aabea..b6b4f2a7e4 100644
--- a/datafusion/functions-aggregate/src/regr.rs
+++ b/datafusion/functions-aggregate/src/regr.rs
@@ -20,7 +20,7 @@
use arrow::datatypes::FieldRef;
use arrow::{array::ArrayRef, datatypes::DataType, datatypes::Field};
use datafusion_common::cast::{as_float64_array, as_uint64_array};
-use datafusion_common::{HashMap, Result, ScalarValue};
+use datafusion_common::{HashMap, Result, ScalarValue, not_impl_err};
use datafusion_doc::aggregate_doc_sections::DOC_SECTION_STATISTICAL;
use datafusion_expr::DistinctHandling;
use datafusion_expr::function::{AccumulatorArgs, StateFieldsArgs};
@@ -469,7 +469,13 @@ impl AggregateUDFImpl for Regr {
self.regr_type != RegrType::Count
}
- fn accumulator(&self, _acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> {
+ if acc_args.is_distinct {
+ return not_impl_err!(
+ "{}(DISTINCT) aggregations are not available",
+ self.func_name.to_uppercase()
+ );
+ }
Ok(Box::new(RegrAccumulator::try_new(&self.regr_type)?))
}
@@ -516,9 +522,9 @@ impl AggregateUDFImpl for Regr {
}
fn distinct_handling(&self) -> DistinctHandling {
- // Duplicate-sensitive, but the accumulator does not read
- // `is_distinct` and today silently returns the non-distinct answer.
- // The tag records the intent; enforcement is a follow-up change.
+ // The accumulator rejects `DISTINCT` with `not_impl_err!`. It only
+ // works when the planner removes it first (`SingleDistinctToGroupBy`
+ // for a single distinct field, e.g. `regr_slope(DISTINCT x, x)`).
DistinctHandling::Unsupported
}
}
diff --git a/datafusion/sqllogictest/test_files/aggregate.slt b/datafusion/sqllogictest/test_files/aggregate.slt
index c1eca4f223..356f0b705e 100644
--- a/datafusion/sqllogictest/test_files/aggregate.slt
+++ b/datafusion/sqllogictest/test_files/aggregate.slt
@@ -432,6 +432,31 @@ GROUP BY g
3 NULL
4 NULL
+# With two different arguments there is no single field to group by, so
+# DISTINCT reaches the accumulator, which does not implement it.
+statement error DataFusion error: This feature is not implemented: CORR\(DISTINCT\) aggregations are not available
+SELECT corr(DISTINCT x, y) FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
+statement error DataFusion error: This feature is not implemented: CORR\(DISTINCT\) aggregations are not available
+SELECT g, corr(DISTINCT x, y) FROM (VALUES (1, 1.0, 1.0), (1, 1.0, 1.0), (1, 2.0, 3.0)) t(g, x, y) GROUP BY g
+
+statement error DataFusion error: This feature is not implemented: COVAR_SAMP\(DISTINCT\) aggregations are not available
+SELECT covar_samp(DISTINCT x, y) FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
+statement error DataFusion error: This feature is not implemented: COVAR_POP\(DISTINCT\) aggregations are not available
+SELECT covar_pop(x, y), covar_pop(DISTINCT x, y) FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
+statement error DataFusion error: This feature is not implemented: REGR_COUNT\(DISTINCT\) aggregations are not available
+SELECT regr_count(x, y), regr_count(DISTINCT x, y) FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
+# Window functions build the accumulator through `create_sliding_accumulator`,
+# which must reject DISTINCT too.
+statement error DataFusion error: This feature is not implemented: CORR\(DISTINCT\) aggregations are not available
+SELECT corr(DISTINCT x, y) OVER () FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
+statement error DataFusion error: This feature is not implemented: REGR_SLOPE\(DISTINCT\) aggregations are not available
+SELECT regr_slope(DISTINCT x, y) OVER () FROM (VALUES (1.0, 1.0), (1.0, 1.0), (2.0, 3.0)) t(x, y)
+
# all_nulls_query_correlation
query R
with data as (
diff --git a/datafusion/sqllogictest/test_files/eliminate_join_distinct.slt b/datafusion/sqllogictest/test_files/eliminate_join_distinct.slt
index 006cad921f..eb9e034872 100644
--- a/datafusion/sqllogictest/test_files/eliminate_join_distinct.slt
+++ b/datafusion/sqllogictest/test_files/eliminate_join_distinct.slt
@@ -187,11 +187,10 @@ SELECT COUNT(DISTINCT l.x), COUNT(l.g) FROM l JOIN r ON l.id = r.id;
----
2 7
-# REGR_COUNT declares `DistinctHandling::Unsupported`: its accumulator ignores
-# `is_distinct` and counts every joined row, so the results below are the
-# non-distinct counts (4 and 6; the distinct counts would be 2 and 4). DISTINCT
-# therefore does not hide the join fanout: the inner join must stay an inner
-# join, and the left join must be kept.
+# REGR_COUNT declares `DistinctHandling::Unsupported`: its accumulator rejects
+# `is_distinct`, so the queries below fail instead of returning the non-distinct
+# counts. DISTINCT therefore does not hide the join fanout: the inner join must
+# stay an inner join, and the left join must be kept.
query TT
EXPLAIN SELECT REGR_COUNT(DISTINCT l.x, l.g) FROM l JOIN r ON l.id = r.id;
----
@@ -202,15 +201,11 @@ logical_plan
04)------TableScan: l projection=[id, g, x]
05)------TableScan: r projection=[id]
-query I
+statement error DataFusion error: This feature is not implemented: REGR_COUNT\(DISTINCT\) aggregations are not available
SELECT REGR_COUNT(DISTINCT l.x, l.g) FROM l JOIN r ON l.id = r.id;
-----
-4
-query I
+statement error DataFusion error: This feature is not implemented: REGR_COUNT\(DISTINCT\) aggregations are not available
SELECT REGR_COUNT(DISTINCT l.x, l.g) FROM l LEFT JOIN r ON l.id = r.id;
-----
-6
# A limit below the aggregate still observes repeated join rows.
query I
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org