Some aggregates silently ignore IGNORE NULLS and ORDER BY on arguments

@jayzhan211

Describe the bug

DataFusion now supports providing ordering and nulls information to aggregates, but some aggregates ignore the flags silently

For example

❯ select first_value(column1 ORDER BY column2) FROM (values (1,2), (3,4), (-1,0)) ;
+----------------------+
| FIRST_VALUE(column1) |
+----------------------+
| -1                   |
+----------------------+

❯ select first_value(column1 ORDER BY column2) IGNORE NULLS FROM (values (1,2), (3,4), (null,0)) ;
+----------------------+
| FIRST_VALUE(column1) |
+----------------------+
| 1                    |
+----------------------+
1 row in set. Query took 0.002 seconds.

To Reproduce

❯ select count(*) from (values (1), (null), (2));
+----------+
| COUNT(*) |
+----------+
| 3        |
+----------+
1 row in set. Query took 0.039 seconds.

❯ select count(*) IGNORE NULLS from (values (1), (null), (2));
+----------+
| COUNT(*) |
+----------+
| 3        |
+----------+
1 row in set. Query took 0.001 seconds.

Also, for ordering

❯ select avg(column1 ORDER BY column2) FROM (values (1,2), (3,4), (null,0)) ;
+--------------+
| AVG(column1) |
+--------------+
| 2.0          |
+--------------+
1 row in set. Query took 0.008 seconds.

Expected behavior

I expect

❯ select count(*) IGNORE NULLS from (values (1), (null), (2));

To error with "IGNORE NULLS is not supported

I also expect

❯ select avg(column1 ORDER BY column2) FROM (values (1,2), (3,4), (null,0)) ;

to error with "ORDER BY" not supported for avg

Additional context

@jayzhan211 has some good ideas at #9920 (comment) about how to make checking this easier / harder to miss

@huaxingao

@huaxingao this might be interesting

@comphead

@comphead Thanks for pinging me! I will work on this issue.

I think for AVG, or COUNT type of aggregates we can safely ignore ORDER BY expression. Since operation result is same for any permutation of the input. However, for first_value, nth_value, last_value this is not the case. However, I agree that IGNORE NULLS should raise an error.

I think for AVG, or COUNT type of aggregates we can safely ignore ORDER BY expression. Since operation result is same for any permutation of the input.

That is true, though applying an ORDER BY to the argument of COUNT may be much slower 🤔 (as DataFusion would have to sort the input)

However, we could remove ORDER BY that don't change the output as a follow on optimization

@jayzhan211

FWIW @jayzhan211 has a really nice API suggestion here I think https://github.com/apache/arrow-datafusion/pull/9920/files#r1549905825

Specifically, add these two functions to the AggregateUDFImpl

/// Return true if the aggregate supports `IGNORE NULL`s
fn support_ignore_nulls() -> bool  { false }

/// Return true if the aggregate supports and `ORDER BY` to specify its ordering:
/// `SELECT first_value(x ORDER BY y) ... `
fn support_ordering() -> bool { false }

@huaxingao

I wrote some tests for this feature in another PR that we decided not to use. I put them here #9953 in case that is helpful @huaxingao

@jayzhan211

FWIW @jayzhan211 has a really nice API suggestion here I think https://github.com/apache/arrow-datafusion/pull/9920/files#r1549905825

Specifically, add these two functions to the AggregateUDFImpl
/// Return true if the aggregate supports `IGNORE NULL`s
fn support_ignore_nulls() -> bool  { false }

/// Return true if the aggregate supports and `ORDER BY` to specify its ordering:
/// `SELECT first_value(x ORDER BY y) ... `
fn support_ordering() -> bool { false }

I think these APIs are very nice, clear. We can add these supports

I opened up an issue that's potentially related to this when it comes to supporting IGNORE NULLS
#15006

Change default of `AggregateUDFImpl::supports_null_handling_clause` t…

Navigation Menu

Repository navigation

Some aggregates silently ignore `IGNORE NULLS` and `ORDER BY` on arguments #9924

Describe the bug

To Reproduce

Expected behavior

Additional context

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Some aggregates silently ignore IGNORE NULLS and ORDER BY on arguments #9924

Description

Describe the bug

To Reproduce

Expected behavior

Additional context

Activity

comphead commented on Apr 3, 2024

huaxingao commented on Apr 3, 2024

mustafasrepo commented on Apr 4, 2024

alamb commented on Apr 4, 2024

alamb commented on Apr 4, 2024

alamb commented on Apr 4, 2024

mustafasrepo commented on Apr 5, 2024

vbarua commented on Mar 4, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Some aggregates silently ignore `IGNORE NULLS` and `ORDER BY` on arguments #9924