[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 31 Oct 2020 05:19:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. IMPALA-10295: fix analytic limit pushdown with no predicates This handles the first case where analytic limit pushdown could be applied incorrectly: when there are no predicates applied to the output of the analytic. If no rows are filtered out between the pre-analytic sort and the place where the top-N will be inserted, and the order matches exactly, we can push down the limit safely because the limit below the analytic will filter exactly the same rows as the limit above the analytic would. We add a helper to check if the sort order matches exactly and then handle the case with no select node correctly. We leave the other cases where there is a special predicate to be handled in the next patch of the series, as the logic there is a bit more subtle. Tests: Added regression planner and query tests that demonstrate the problem. Ran core tests. Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Reviewed-on: http://gerrit.cloudera.org:8080/16663 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test M testdata/workloads/tpch/queries/limit-pushdown-analytic.test 3 files changed, 238 insertions(+), 39 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6626/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 31 Oct 2020 00:03:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 31 Oct 2020 00:03:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Oct 2020 23:48:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 4: Code-Review+1 carry +1 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Oct 2020 23:40:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7586/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Oct 2020 00:35:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Hello Aman Sinha, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16663 to look at the new patch set (#4). Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. IMPALA-10295: fix analytic limit pushdown with no predicates This handles the first case where analytic limit pushdown could be applied incorrectly: when there are no predicates applied to the output of the analytic. If no rows are filtered out between the pre-analytic sort and the place where the top-N will be inserted, and the order matches exactly, we can push down the limit safely because the limit below the analytic will filter exactly the same rows as the limit above the analytic would. We add a helper to check if the sort order matches exactly and then handle the case with no select node correctly. We leave the other cases where there is a special predicate to be handled in the next patch of the series, as the logic there is a bit more subtle. Tests: Added regression planner and query tests that demonstrate the problem. Ran core tests. Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 --- M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test M testdata/workloads/tpch/queries/limit-pushdown-analytic.test 3 files changed, 238 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/16663/4 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16663/3/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/16663/3/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@22 PS3, Line 22: import java.util.logging.Logger; > nit: for some reason this changed from slf4j to the java logger. Done -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Oct 2020 00:10:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/16663/2/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/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@478 PS2, Line 478: if (!analyticSortExprs.get(i).equals(sortExprs.get(i))) return false; > It returns true in some circumstances. That particular example didn't consi Done http://gerrit.cloudera.org:8080/#/c/16663/3/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/16663/3/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@22 PS3, Line 22: import java.util.logging.Logger; nit: for some reason this changed from slf4j to the java logger. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Oct 2020 22:07:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/7581/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Oct 2020 18:06:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16663 )
Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/16663/2/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/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@478
PS2, Line 478: if (!analyticSortExprs.get(i).equals(sortExprs.get(i)))
return false;
> Will this comparison return True if for example both the analytic sort expr
It returns true in some circumstances. That particular example didn't consider
them equal because we materialized the order by expression into a slot of the
sort tuple, and one of the exprs referenced that slot and the other referenced
the original slot.
So this is now slightly more general than the previous variant, which only
handled bare SlotRefs, but it doesn't handle all cases where the optimisation
might be valid.
http://gerrit.cloudera.org:8080/#/c/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@480
PS2, Line 480: if
(!sortInfo.getIsAscOrder().get(i).equals(analyticSortInfo.getIsAscOrder().get(i)))
{
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@483
PS2, Line 483: if
(!sortInfo.getNullsFirst().get(i).equals(analyticSortInfo.getNullsFirst().get(i)))
{
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/16663/2/testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test
File
testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test:
http://gerrit.cloudera.org:8080/#/c/16663/2/testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test@213
PS2, Line 213: # Limit pushdown should be applied
> Change this comment according to the new behavior.
Done
--
To view, visit http://gerrit.cloudera.org:8080/16663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21
Gerrit-Change-Number: 16663
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 29 Oct 2020 17:53:58 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Hello Aman Sinha, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16663 to look at the new patch set (#3). Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. IMPALA-10295: fix analytic limit pushdown with no predicates This handles the first case where analytic limit pushdown could be applied incorrectly: when there are no predicates applied to the output of the analytic. If no rows are filtered out between the pre-analytic sort and the place where the top-N will be inserted, and the order matches exactly, we can push down the limit safely because the limit below the analytic will filter exactly the same rows as the limit above the analytic would. We add a helper to check if the sort order matches exactly and then handle the case with no select node correctly. We leave the other cases where there is a special predicate to be handled in the next patch of the series, as the logic there is a bit more subtle. Tests: Added regression planner and query tests that demonstrate the problem. Ran core tests. Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 --- M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test M testdata/workloads/tpch/queries/limit-pushdown-analytic.test 3 files changed, 238 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/16663/3 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16663/2/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/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@478 PS2, Line 478: if (!analyticSortExprs.get(i).equals(sortExprs.get(i))) return false; Will this comparison return True if for example both the analytic sort expr and sort expr is the same function ..CAST(a as int) ? I notice one of your tests does a lower() function in both exprs. I would think that it should satisfy the equality check but it doesn't ? http://gerrit.cloudera.org:8080/#/c/16663/2/testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test File testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test: http://gerrit.cloudera.org:8080/#/c/16663/2/testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test@213 PS2, Line 213: # Limit pushdown should be applied Change this comment according to the new behavior. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 28 Oct 2020 17:29:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7568/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 27 Oct 2020 17:05:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16663 ) Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7567/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 27 Oct 2020 17:05:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/16663 )
Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16663/2/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/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@480
PS2, Line 480: if
(!sortInfo.getIsAscOrder().get(i).equals(analyticSortInfo.getIsAscOrder().get(i)))
{
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16663/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@483
PS2, Line 483: if
(!sortInfo.getNullsFirst().get(i).equals(analyticSortInfo.getNullsFirst().get(i)))
{
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/16663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21
Gerrit-Change-Number: 16663
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 27 Oct 2020 16:45:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/16663 )
Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16663/1/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/16663/1/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@478
PS1, Line 478: if
(!sortInfo.getIsAscOrder().get(i).equals(analyticSortInfo.getIsAscOrder().get(i)))
{
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16663/1/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@481
PS1, Line 481: if
(!sortInfo.getNullsFirst().get(i).equals(analyticSortInfo.getNullsFirst().get(i)))
{
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/16663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21
Gerrit-Change-Number: 16663
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 27 Oct 2020 16:44:41 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Hello Aman Sinha, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16663 to look at the new patch set (#2). Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. IMPALA-10295: fix analytic limit pushdown with no predicates This handles the first case where analytic limit pushdown could be applied incorrectly: when there are no predicates applied to the output of the analytic. If no rows are filtered out between the pre-analytic sort and the place where the top-N will be inserted, and the order matches exactly, we can push down the limit safely because the limit below the analytic will filter exactly the same rows as the limit above the analytic would. We add a helper to check if the sort order matches exactly and then handle the case with no select node correctly. We leave the other cases where there is a special predicate to be handled in the next patch of the series, as the logic there is a bit more subtle. Tests: Added regression planner and query tests that demonstrate the problem. Ran core tests. Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 --- M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test M testdata/workloads/tpch/queries/limit-pushdown-analytic.test 3 files changed, 234 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/16663/2 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10295: fix analytic limit pushdown with no predicates
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16663 Change subject: IMPALA-10295: fix analytic limit pushdown with no predicates .. IMPALA-10295: fix analytic limit pushdown with no predicates This handles the first case where analytic limit pushdown could be applied incorrectly: when there are no predicates applied to the output of the analytic. If no rows are filtered out between the pre-analytic sort and the place where the top-N will be inserted, and the order matches exactly, we can push down the limit safely because the limit below the analytic will filter exactly the same rows as the limit above the analytic would. We add a helper to check if the sort order matches exactly and then handle the case with no select node correctly. We leave the other cases where there is a special predicate to be handled in the next patch of the series, as the logic there is a bit more subtle. Tests: Added regression planner and query tests that demonstrate the problem. Ran core tests. Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 --- M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M testdata/workloads/functional-planner/queries/PlannerTest/limit-pushdown-analytic.test M testdata/workloads/tpch/queries/limit-pushdown-analytic.test 3 files changed, 232 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/16663/1 -- To view, visit http://gerrit.cloudera.org:8080/16663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I254e85edd5ea6b6e76d20cbdf27fd88059a98a21 Gerrit-Change-Number: 16663 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
