Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21976 )

Change subject: IMPALA-13481: Add support for various agg and analytic functions
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21976/9/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RewriteRexOverRule.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RewriteRexOverRule.java:

http://gerrit.cloudera.org:8080/#/c/21976/9/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RewriteRexOverRule.java@118
PS9, Line 118:      * cume_dist() over([partition by clause] order by clause)
In Impala's documentation 
(https://impala.apache.org/docs/build/plain-html/topics/impala_analytic_functions.html
 ), we mention that we can reverse the order of the cume_dist by specifying 
"order by X desc". I'm not seeing an existing test case for that (boo!).

I did a hand test using this SQL from analytic-fns.test:
https://github.com/apache/impala/blob/master/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test#L1390-L1427

I added "desc" to the order by for cume_dist. With the regular planner, it 
changed the output compared to without "desc". With Calcite, the output didn't 
change. We'll need to add test cases and fix up that ordering. Something 
similar might apply to ntile() and percent_rank().

It would be ok to split that off as a separate change if it gets complicated.



-- 
To view, visit http://gerrit.cloudera.org:8080/21976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57c69a60c63872b2964688f395b662a85698555e
Gerrit-Change-Number: 21976
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Tue, 19 Nov 2024 18:56:01 +0000
Gerrit-HasComments: Yes

Reply via email to