Skip to content

fix: Coerce aggregate FILTER predicates to boolean #22774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pchintar wants to merge 1 commit into
base: main
Choose a base branch
from
+45 −0
Open

fix: Coerce aggregate FILTER predicates to boolean #22774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions datafusion/optimizer/src/analyzer/type_coercion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,12 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
}) => {
let new_expr =
coerce_arguments_for_signature(args, self.schema, func.as_ref())?;

let filter = filter
.map(|filter| filter.cast_to(&DataType::Boolean, self.schema))
.transpose()?
.map(Box::new);

Ok(Transformed::yes(Expr::AggregateFunction(
expr::AggregateFunction::new_udf(
func,
Expand Down Expand Up @@ -752,6 +758,11 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
}
};

let filter = filter
.map(|filter| filter.cast_to(&DataType::Boolean, self.schema))
.transpose()?
.map(Box::new);

let new_expr = Expr::from(WindowFunction {
fun,
params: expr::WindowFunctionParams {
Expand Down
34 changes: 34 additions & 0 deletions datafusion/sqllogictest/test_files/aggregate.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6266,6 +6266,40 @@ GROUP BY g
----
0 0

# query_with_untyped_null_filter
query I
SELECT count(*) FILTER (WHERE NULL)
----
0

query I
SELECT count(1) FILTER (WHERE NULL)
----
0

query I
SELECT sum(1) FILTER (WHERE NULL)
----
NULL

query R
SELECT avg(1) FILTER (WHERE NULL)
----
NULL

# window_aggregate_with_untyped_null_filter
query I
SELECT count(*) FILTER (WHERE NULL) OVER ()
FROM (VALUES (1)) AS t(x)
----
0

query I
SELECT sum(1) FILTER (WHERE NULL) OVER ()
FROM (VALUES (1)) AS t(x)
----
NULL

Comment on lines +6290 to +6302
Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be moved to window.slt? On the other hand there are already other window function tests at aggregate.slt, so I'm not sure what is the criteria.

Copy link
Copy Markdown
Author

@pchintar pchintar Jun 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nuno-faria , and thanks for checking the predicate behavior against DuckDB/Postgres too.

I put these in aggregate.slt because the failure is in aggregate FILTER handling and there are already related aggregate/window aggregate cases here. So, Keeping the new cases alongside the existing aggregate FILTER coverage felt more consistent to me.

# query_with_and_without_filter
query III rowsort
SELECT
Expand Down
Loading