impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Qifan Chen (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-9983 : Pushdown limit to analytic sort operator
Date Wed, 29 Jul 2020 16:52:32 GMT
Qifan Chen has posted comments on this change. ( )

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

Patch Set 10:


Looks good to me!
File fe/src/main/java/org/apache/impala/planner/
PS10, Line 406: partitionExprs_.size() > sortExprs.size()
Will this be a possibility?

sort on a
partition on (a,1)
PS10, Line 416: SlotRef
Not clear on this. I thought SlotRef is useful when dealing with run-time property of an expression.
PS10, Line 431: !sortInfo.getIsAscOrder().get(i) 
I wonder if this check is necessary. If the sort order is descending, then if the same order
is applied to the split version of the sort instances, push-down LIMIT still works.

Assume the pushdown works by asking the child sort operators to produce at least <n>
rows. where <n> is the limit at the query level.
PS10, Line 481: r =, <, <= 
Not sure the rational behind it.
PS10, Line 487: rhs of the predic
Not follow.
File fe/src/main/java/org/apache/impala/planner/
PS10, Line 343: offset <= 0
Thought that offset is non-negative in SQL.
File fe/src/main/java/org/apache/impala/planner/
PS10, Line 143: Preconditions.checkArgument(type_ == TSortType.TOTAL);
If this is a TopN sort, then the method should succeed if both the limit and partitioningExprs
in args and contained within this are the same.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib39f46a7bb75a34466eef7f91ddc25b6e6c99284
Gerrit-Change-Number: 16219
Gerrit-PatchSet: 10
Gerrit-Owner: Aman Sinha <>
Gerrit-Reviewer: Aman Sinha <>
Gerrit-Reviewer: David Rorke <>
Gerrit-Reviewer: Impala Public Jenkins <>
Gerrit-Reviewer: Qifan Chen <>
Gerrit-Reviewer: Shant Hovsepian <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 29 Jul 2020 16:52:32 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message