Aggregate window functions do not support IGNORE NULLS

facebookincubator / velox

A C++ vectorized database acceleration library aimed to optimizing query engines and data processing systems.

https://velox-lib.io/

Apache License 2.0

3.42k stars 1.12k forks source link

Aggregate window functions do not support IGNORE NULLS #9385

Open mbasmanova opened 5 months ago

mbasmanova commented 5 months ago

Bug description

"sum"(x) IGNORE NULLS OVER (
          PARTITION BY a 
          ORDER BY 
            b ASC ROWS BETWEEN 83 PRECEDING 
            AND CURRENT ROW
        )

VeloxUserError: !ignoreNulls Aggregate window functions do not support IGNORE NULLS

Meta query sample: 20240405_000418_00069_vscy5

CC: @kagamiori @aditi-pandit @amitkdutta

System information

n/a

Relevant logs

No response

aditi-pandit commented 5 months ago

@mbasmanova, @kagamiori : We have been discussing Presto java behavior for these queries in https://github.com/prestodb/presto/issues/21304.

In summary, IGNORE NULLs seems like a no-op in Presto java unless I'm misunderstaning. So I feel the Prestissimo behavior is better. Would be great to hear your thoughts.

mbasmanova commented 5 months ago

@aditi-pandit Aditi, thank you for looking. I see that Presto simply ignores 'IGNORE NULLS' setting when using aggregate function in a window operator. This is hacky, but I guess it works most of the time because most aggregate functions ignore null inputs anyway.

It would be nice to find out where Presto's behavior matches SQL spec. If it does, then we'll need to fix Velox to also ignore 'IGNORE NULLS' setting for aggregate functions.