You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2021/06/22 06:23:43 UTC
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17615
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
IMPALA-10755: Fix migration of analytic predicate to inline view.
As part of IMPALA-9979 we made changes to push down predicates
that reference analytic tuple into the inline view. In cases where
both sides of a predicate are slot references (for example,
a = MAX(b) where MAX(b) is an analytic function), it may not be
safe to push it into the inline view since the two sides may be
referencing separate tuples.
This patch fixes the behavior by skipping such predicates such
that they will be left unassigned and will subsequently get
assigned to a SELECT node above the analytic operator.
Testing:
- Added planner tests for analytic predicates ensuring that
analytic predicates are present in the SELECT node.
- Added run time tests for the same using TPC-H and
verified correctness.
Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
A testdata/workloads/tpch/queries/analytic-fns.test
M tests/query_test/test_queries.py
4 files changed, 207 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17615/1
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 2:
(7 comments)
> Patch Set 1:
>
> (1 comment)
Thanks Quanlong for the quick review. Pls see responses.
http://gerrit.cloudera.org:8080/#/c/17615/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17615/1//COMMIT_MSG@20
PS1, Line 20: Testing:
> TODO: run all FE and relevant end-to-end tests.
Done
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1397
PS1, Line 1397: Expr viewPred = pred.substitute(inlineViewRef.getSmap(), analyzer, false);
: tupleIds.clear();
> nit: this can be replaced as
Done
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1402
PS1, Line 1402: ere MAX
> nit: this can be null since we don't need the slotIds.
Done
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1408
PS1, Line 1408: }
> nit: Can we simply use isBound()? i.e.
I did not change this because after looking at the implementation of SlotRef.isBoundByTupleIds() and SlotRef.referencesTuple(). I see there's an extra Preconditions check in referencesTuple:
Preconditions.checkState(type_.isValid());
I think it is useful to do that check, so I left this as-is. Let me know if you think otherwise.
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test
File testdata/workloads/tpch/queries/analytic-fns.test:
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@23
PS1, Line 23: ---- RESULTS
> nit: we don't need the ORDER BY since the RESULTS section is not specified
Makes sense. Done.
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@52
PS1, Line 52: ULTS
> nit: could you cast this to BIGINT? Just concerning if this will be flaky s
Good point. Added the cast.
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@53
PS1, Line 53: 1.00,1,11,7,12
> nit: we don't need the ORDER BY since the RESULTS section is not specified
Done
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 23:18:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
LGTM. Bump to +2 since the GVO passed. Thanks for fixing this quickly!
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1408
PS1, Line 1408: }
> I did not change this because after looking at the implementation of SlotRe
I see. Thanks for the explanation!
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py@367
PS2, Line 367: class TestAnalyticFnsTpch(ImpalaTestSuite):
> flake8: E302 expected 2 blank lines, found 1
nit: we need one more blank line above.
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 24 Jun 2021 00:38:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7254/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 10:13:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8965/ : 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/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jun 2021 06:44:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1: Code-Review+1
(6 comments)
Thanks for fixing this quickly! I can bump my +1 to +2 once it passes CORE tests. Just left some minor comments.
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1397
PS1, Line 1397: for (int i = 0; i < conjuncts.size(); ++i) {
: Expr pred = conjuncts.get(i);
nit: this can be replaced as
for (Expr pred : conjuncts) {
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1402
PS1, Line 1402: slotIds
nit: this can be null since we don't need the slotIds.
http://gerrit.cloudera.org:8080/#/c/17615/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1408
PS1, Line 1408: if (viewPred.referencesTuple(analyticTuple.getId()) && tupleIds.size() <= 1) {
nit: Can we simply use isBound()? i.e.
viewPred.isBound(analyticTuple.getId())
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test
File testdata/workloads/tpch/queries/analytic-fns.test:
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@23
PS1, Line 23: ORDER BY 1, 2, 3, 4
nit: we don't need the ORDER BY since the RESULTS section is not specified with VERIFY_IS_EQUAL_SORTED
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@52
PS1, Line 52: avg_nr_pvp
nit: could you cast this to BIGINT? Just concerning if this will be flaky since it's comparing DOUBLE with BIGINT.
cast(avg_nr_pvp as bigint) = max_nr_pvp - 5
http://gerrit.cloudera.org:8080/#/c/17615/1/testdata/workloads/tpch/queries/analytic-fns.test@53
PS1, Line 53: ORDER BY 1, 2, 3, 4, 5
nit: we don't need the ORDER BY since the RESULTS section is not specified with VERIFY_IS_EQUAL_SORTED
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 06:52:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17615
to look at the new patch set (#3).
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
IMPALA-10755: Fix migration of analytic predicate to inline view.
As part of IMPALA-9979 we made changes to push down predicates
that reference analytic tuple into the inline view. In cases where
both sides of a predicate are slot references (for example,
a = MAX(b) where MAX(b) is an analytic function), it may not be
safe to push it into the inline view since the two sides may be
referencing separate tuples.
This patch fixes the behavior by skipping such predicates such
that they will be left unassigned and will subsequently get
assigned to a SELECT node above the analytic operator.
Testing:
- Added planner tests for analytic predicates ensuring that
analytic predicates are present in the SELECT node.
- Added run time tests for the same using TPC-H and
verified correctness.
Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
A testdata/workloads/tpch/queries/analytic-fns.test
M tests/query_test/test_queries.py
4 files changed, 205 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17615/3
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17615/1/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/17615/1/tests/query_test/test_queries.py@367
PS1, Line 367: class TestAnalyticFnsTpch(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jun 2021 06:24:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 16:09:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py@367
PS2, Line 367: class TestAnalyticFnsTpch(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 23:07:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
IMPALA-10755: Fix migration of analytic predicate to inline view.
As part of IMPALA-9979 we made changes to push down predicates
that reference analytic tuple into the inline view. In cases where
both sides of a predicate are slot references (for example,
a = MAX(b) where MAX(b) is an analytic function), it may not be
safe to push it into the inline view since the two sides may be
referencing separate tuples.
This patch fixes the behavior by skipping such predicates such
that they will be left unassigned and will subsequently get
assigned to a SELECT node above the analytic operator.
Testing:
- Added planner tests for analytic predicates ensuring that
analytic predicates are present in the SELECT node.
- Added run time tests for the same using TPC-H and
verified correctness.
Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Reviewed-on: http://gerrit.cloudera.org:8080/17615
Reviewed-by: Aman Sinha <am...@cloudera.com>
Tested-by: Aman Sinha <am...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
A testdata/workloads/tpch/queries/analytic-fns.test
M tests/query_test/test_queries.py
4 files changed, 205 insertions(+), 5 deletions(-)
Approvals:
Aman Sinha: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 3: Verified+1 Code-Review+2
(1 comment)
Carry +2 from Quanlong.
Adding +1 for Verified based on successful GVO run.
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/17615/2/tests/query_test/test_queries.py@367
PS2, Line 367:
> nit: we need one more blank line above.
Done
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 24 Jun 2021 01:31:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8988/ : 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/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 23:27:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8990/ : 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/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 24 Jun 2021 01:42:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17615 )
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17615/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17615/1//COMMIT_MSG@20
PS1, Line 20: Testing:
TODO: run all FE and relevant end-to-end tests.
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jun 2021 06:27:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10755: Fix migration of analytic predicate to inline view.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17615
to look at the new patch set (#2).
Change subject: IMPALA-10755: Fix migration of analytic predicate to inline view.
......................................................................
IMPALA-10755: Fix migration of analytic predicate to inline view.
As part of IMPALA-9979 we made changes to push down predicates
that reference analytic tuple into the inline view. In cases where
both sides of a predicate are slot references (for example,
a = MAX(b) where MAX(b) is an analytic function), it may not be
safe to push it into the inline view since the two sides may be
referencing separate tuples.
This patch fixes the behavior by skipping such predicates such
that they will be left unassigned and will subsequently get
assigned to a SELECT node above the analytic operator.
Testing:
- Added planner tests for analytic predicates ensuring that
analytic predicates are present in the SELECT node.
- Added run time tests for the same using TPC-H and
verified correctness.
Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
A testdata/workloads/tpch/queries/analytic-fns.test
M tests/query_test/test_queries.py
4 files changed, 204 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17615/2
--
To view, visit http://gerrit.cloudera.org:8080/17615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5cad3d408ee3695cafb35f66a4f19b4e8d0529e
Gerrit-Change-Number: 17615
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>