Regression in `last_value` functionality

[andygrove]

Describe the bug

In the Comet subproject, we have a PR where we are upgrading to use the latest DataFusion revision from main in preparation for upgrading to DataFusion 47 (apache/datafusion-comet#1563).

After bumping the revision to b22e4d26480202247022bf2c94ba629a07e3275b, we see some tests start to fail, such as:

multiple group-by columns + single aggregate column, with nulls

2025-04-10T17:13:49.0990050Z   == Results ==
2025-04-10T17:13:49.0990580Z   !== Correct Answer - 2 ==                        == Spark Answer - 2 ==
2025-04-10T17:13:49.0991090Z    struct<_g1:tinyint,_g2:smallint,last(_3):int>   struct<_g1:tinyint,_g2:smallint,last(_3):int>
2025-04-10T17:13:49.0991750Z    [null,null,null]                                [null,null,null]
2025-04-10T17:13:49.0992100Z   ![0,0,9]                                         [0,0,0] (QueryTest.scala:244)

It is also possible that the error is due to other changes in Comet and not related to DataFusion. I will update this issue once I know more.

To Reproduce

No response

Expected behavior

No response

Additional context

No response

[andygrove]

The issue starts happening with #15542, which seems to have changed the behavior of the last_value aggregate. I am still investigating.

[andygrove]

The following query previously produced different values for FIRST and LAST but now produces the same value for both.

SELECT _g1, _g2, FIRST(_3), LAST(_3) FROM v GROUP BY _g1, _g2 ORDER BY _g1, _g2

Before:

[0,0,0,9] 

After

[0,0,0,0] 

[andygrove]

I confirmed that the behavior in DataFusion also changed when #15542 was merged.

Before

query III
SELECT y, FIRST_VALUE(x), LAST_VALUE(x)
FROM FOO
GROUP BY y
ORDER BY y;
----
2 1 1
3 2 1

After

query III
SELECT y, FIRST_VALUE(x), LAST_VALUE(x)
FROM FOO
GROUP BY y
ORDER BY y;
----
2 1 1
3 2 2

[andygrove]

@UBarney fyi

[Dandandan]

I wonder, why do we even support LAST_VALUE / FIRST_VALUE as plain aggregate functions?
I think hey don't make any sense as aggregate functions (but only as window functions).

[Dandandan]

(But I think we should of course fix this regression).

[andygrove]

I wonder, why do we even support LAST_VALUE / FIRST_VALUE as plain aggregate functions? I think hey don't make any sense as aggregate functions (but only as window functions).

They are valid in Spark as plain aggregates. I'm not sure if any other query engines support this use.

[UBarney]

I implemented it according to the behavior defined in this document

Returns the last element in an aggregation group according to the requested ordering. If no ordering is given, returns an arbitrary element from the group.

https://datafusion.apache.org/user-guide/sql/aggregate_functions.html#last-value

[jayzhan211]

I implemented it according to the behavior defined in this document

Returns the last element in an aggregation group according to the requested ordering. If no ordering is given, returns an arbitrary element from the group.

https://datafusion.apache.org/user-guide/sql/aggregate_functions.html#last-value

The doc isn't correct, we should return the last value in the group

[jayzhan211]

I wonder, why do we even support LAST_VALUE / FIRST_VALUE as plain aggregate functions? I think hey don't make any sense as aggregate functions (but only as window functions).

DuckDb has first last too, I guess it would be useful in some cases? 🤔