Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16219 )

Change subject: IMPALA-9983 : Pushdown limit to analytic sort operator
......................................................................


Patch Set 12:

(4 comments)

> Patch Set 8:
>
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/16219/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:

http://gerrit.cloudera.org:8080/#/c/16219/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@406
PS10, Line 406: f there is no sort expr in the parent sor
> Will this be a possibility?
It is valid but I would guess quite rare to see constants in the partition-by 
(although i guess BI tools generated all kinds of weird queries, so it may 
occur there).  In any case, I think for this patch, it is better to restrict 
the applicability.


http://gerrit.cloudera.org:8080/#/c/16219/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@416
PS10, Line 416:
> Not clear on this. I thought SlotRef is useful when dealing with run-time p
Can you elaborate on the 'run-time property' part ? The question wasn't quite 
clear to me.


http://gerrit.cloudera.org:8080/#/c/16219/8/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
File fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java:

http://gerrit.cloudera.org:8080/#/c/16219/8/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java@266
PS8, Line 266:     return createSortInfo(input, sortExprs, isAsc, nullsFirst, 
TSortingOrder.LEXICAL);
> line too long (97 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16219/9/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
File fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java:

http://gerrit.cloudera.org:8080/#/c/16219/9/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java@266
PS9, Line 266:     return createSortInfo(input, sortExprs, isAsc, nullsFirst, 
TSortingOrder.LEXICAL);
> line too long (97 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib39f46a7bb75a34466eef7f91ddc25b6e6c99284
Gerrit-Change-Number: 16219
Gerrit-PatchSet: 12
Gerrit-Owner: Aman Sinha <amsi...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com>
Gerrit-Reviewer: David Rorke <dro...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Shant Hovsepian <sh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jul 2020 07:26:06 +0000
Gerrit-HasComments: Yes

Reply via email to