You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "xqhe (Code Review)" <ge...@cloudera.org> on 2020/01/16 03:42:44 UTC
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
xqhe has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15047
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
IMPALA-8361: Fix bound predicates optimization doesn't work for
InlineView
This is improvement that migrated predicates of the nullable side of
the outer join into inline view.We should duplicate it and evaluate
after join.
For example:
EXPLAIN
SELECT count(*) FROM functional.alltypessmall a
LEFT JOIN
(SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny) b ON a.id=b.id
WHERE b.upper_val='1';
+------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------+
| Max Per-Host Resource Reservation: Memory=1.95MB Threads=5 |
| Per-Host Resource Estimates: Memory=86MB |
| Codegen disabled by planner |
| |
| PLAN-ROOT SINK |
| | |
| 06:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | row-size=8B cardinality=1 |
| | |
| 05:EXCHANGE [UNPARTITIONED] |
| | |
| 03:AGGREGATE |
| | output: count(*) |
| | row-size=8B cardinality=1 |
| | |
| 02:HASH JOIN [LEFT OUTER JOIN, BROADCAST] |
| | hash predicates: a.id = id |
| | other predicates: upper(string_col) = '1' <-eval after join
| | row-size=21B cardinality=100 |
| | |
| |--04:EXCHANGE [BROADCAST] |
| | | |
| | 01:SCAN HDFS [functional.alltypestiny] |
| | HDFS partitions=4/4 files=4 size=460B |
| | predicates: upper(string_col) = '1' <-push down |
| | row-size=17B cardinality=4 |
| | |
| 00:SCAN HDFS [functional.alltypessmall a] |
| HDFS partitions=4/4 files=4 size=6.32KB |
| row-size=4B cardinality=100 |
+------------------------------------------------------------+
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
3 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/2
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 13:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5491/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 13
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 19 Mar 2020 02:16:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5469/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 4
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 19 Jan 2020 12:30:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 12:01:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 16:25:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2:
hexianqing, thanks for your contribution. I will review this over the weekend. BTW, there are PlannerTest failures according to the jenkins run.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jan 2020 18:39:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5425/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jan 2020 03:31:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is an improvement that tries to propagate predicates of the
nullable side of the outer join into inline view.
For example:
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val,
length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0
Before this change, the predicate b.len=0 can't be migrated into inline
view since that is on the nullable side of an outer join if the
predicate evaluates in the inline view nulls will not be rejected.
However, we can be more aggressive. In particular, some predicates that
must be evaluted at a join node can also be safely evaluted by the
outer-joined inline view. Such predicates are not marked as assigned.
The predicates propagate into the inline view and also be evaluated at
a join node.
We can divide predicates into two types. One that satisfies the condition
that same as Analyzer#canEvalPredicate can be migrated into inline view，
and one that satisfies the below three conditions is safe to be propagated
into the nullable side of an outer join.
1) The predicate needs to be bound by tupleIds.
2) The predicate is not on-clause.
3) The predicate evaluates to false when all its referenced tuples are NULL.
Therefore, 'b.upper_val is NULL' cannot be propagated to inline view but
‘b.len = 0’ can be propagated to inline view.
Tests:
* Add plan tests in inline-view.test
* One baseline plan in inline-view.test, one in nested-collections.test
and two in predicate-propagation.test had to be updated
* Ran the full set of verifications in Impala Public Jenkins
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Reviewed-on: http://gerrit.cloudera.org:8080/15047
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
7 files changed, 317 insertions(+), 73 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 14
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is an improvement that tries to propagate predicates of the
nullable side of the outer join into inline view.
For example:
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val,
length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0
Before this change, the predicate b.len=0 can't be migrated into inline
view since that is on the nullable side of an outer join if the
predicate evaluates in the inline view nulls will not be rejected.
However,we can be more aggressive. In particular, the predicates that
must be evaluted at a join node but can also be safely evaluted by the
outer-joined inline view. Such predicates are not marked as assigned.
The predicates propagate into the inline view and also be evaluated at
a join node.
We can divide predicates into two types.One that satisfies the condition
that same as Analyzer#canEvalPredicate can be migrated into inline view，
and one that satisfies the below three conditions is safe to be propagated
into the nullable side of an outer join.
1) The predicate needs to be bound by tupleIds.
2) The predicate is not on-clause.
3) The predicate evaluates to false when all its referenced tuples are NULL.
Therefore, 'b.upper_val is NULL' cannot be propagated to inline view but
‘b.len = 0’ can be propagated to inline view.
Tests:
* Add plan tests in inline-view.test
* One baseline plan in inline-view.test, one in nested-collections.test
and two in predicate-propagation.test had to be updated
* Ran the full set of verifications in Impala Public Jenkins
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
6 files changed, 334 insertions(+), 57 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/9
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5283/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 05 Feb 2020 07:54:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is improvement that propagate predicates of the nullable side of
the outer join into inline view.
I refactored the code "Analyzer#canEvalPredicate()" and add
"SingleNodePlanner#getConjunctsToInlineView()" to get conjucts can be
correctly to migrate or propagate into inline view.
Before this change, we skip predicate b.upper_val='1' since
canEvalPredicate() returns false on it. Because it doesn't satisfy
isLastOjMaterializedByTupleIds(). So we can't migrate it inside the
inline view. However, we can be more aggressive. It's correct to
duplicate(not migrate) this predicate inside the inline view since
it's not evaluted to true with null slots.
This patch picks up the predicates that being skipped due to not
satisfying isLastOjMaterializedByTupleIds() and duplicates them
if they satisfy !isTrueWithNullSlots().
For example:
EXPLAIN
SELECT count(*) FROM functional.alltypessmall a
LEFT JOIN
(SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny) b ON a.id=b.id
WHERE b.upper_val='1';
+------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------+
| Max Per-Host Resource Reservation: Memory=1.95MB Threads=5 |
| Per-Host Resource Estimates: Memory=86MB |
| Codegen disabled by planner |
| |
| PLAN-ROOT SINK |
| | |
| 06:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | row-size=8B cardinality=1 |
| | |
| 05:EXCHANGE [UNPARTITIONED] |
| | |
| 03:AGGREGATE |
| | output: count(*) |
| | row-size=8B cardinality=1 |
| | |
| 02:HASH JOIN [LEFT OUTER JOIN, BROADCAST] |
| | hash predicates: a.id = id |
| | other predicates: upper(string_col) = '1' <-eval after join
| | row-size=21B cardinality=100 |
| | |
| |--04:EXCHANGE [BROADCAST] |
| | | |
| | 01:SCAN HDFS [functional.alltypestiny] |
| | HDFS partitions=4/4 files=4 size=460B |
| | predicates: upper(string_col) = '1' <-push down |
| | row-size=17B cardinality=4 |
| | |
| 00:SCAN HDFS [functional.alltypessmall a] |
| HDFS partitions=4/4 files=4 size=6.32KB |
| row-size=4B cardinality=100 |
+------------------------------------------------------------+
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
6 files changed, 144 insertions(+), 57 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/7
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5352/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 01:43:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5422/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2020 12:36:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 12:
(26 comments)
I'm really sorry that because I'm not familiar with gerrit, I found that none of the replies were sent out. Thank you for reviewing my code. I apologize again for not replying before
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@9
PS7, Line 9: This is an improvement that tries
> "This is an improvement that tries to propagate"
Done
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@11
PS7, Line 11:
: For example:
: SELECT *
: FROM functional.alltypessmall a
: LEFT JOIN (
: SELECT id, upper(string_col) AS upper_val,
: length(string_col) AS len
: FROM functional.alltypestiny
: ) b ON a.id = b.id
> Please take some time to write the commit message. It's very important for
Done
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@11
PS7, Line 11:
: For example:
: SELECT *
: FROM functional.alltypessmall a
: LEFT JOIN (
: SELECT id, upper(string_col) AS upper_val,
: length(string_col) AS len
: FROM functional.alltypestiny
: ) b ON a.id = b.id
> Agree with Quanlong regarding improving the commit message. In addition, I
Done
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: ,
> nit: need space
Done
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: some predicates that
: must be evaluted at a join node can also be safely evaluted by the
: outer-joined inline view.
> I think you mean "some predicates that must be evaluted at a join node can
Done
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@30
PS9, Line 30: .
> nit: need space
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1618
PS7, Line 1618: return true;
> Pls add javadoc comment
Done
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1588
PS11, Line 1588: * - For On-clause predicates, see canEvalOnClauseConjunct() for more info.
> I think this part of the comment about on-clause predicates should be moved
Done
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1643
PS11, Line 1643: e.getIds(exprTids, null);
> Maybe rename this to exprTids or similar, so that it's clearer how it is di
Done
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@617
PS11, Line 617: * returned by getResultTupleId().
> Document that it only returns unique expressions?
If some conjuncts are pushed to having predicates, it may appear duplicate predicates, eg
AGGREGATE [FINALIZE]
| group by: id
| having: id = 12, id = 12
Here's a simple fix for this situation.
http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@985
PS9, Line 985: picked up by getBoundPredicates()
> I think this comment is stale after merging this patch. Could you update it
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/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/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1162
PS7, Line 1162: Get the unassigned conjuncts that can be eva
> "Get the unassigned conjuncts that can be evaluated in inline view and retu
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1163
PS7, Line 1163: * in 'evalInInlineViewPreds'.
: * If a conjunct is not an On-clause predicate and is safe to
> If a conjunct is not an On-clause predicate and is safe to propagate it ins
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1167
PS7, Line 1167: er an
> I think something like "evalInViewPreds" may be more clear for its meaning.
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1176
PS7, Line 1176: e.getIds(tids, null);
: if (tids.isEmpt
> Just be curious, can we add test coverage for this case?
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1190
PS7, Line 1190: r
> nit: remove space here
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1203
PS7, Line 1203:
> nit: remove space here
Done
http://gerrit.cloudera.org:8080/#/c/15047/10/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/15047/10/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1178
PS10, Line 1178: evalInInlineViewPreds.add(e);
> Can we simply fix IMPALA-9356 by adding this to evalAfterJoinPreds?
For inline view we can fix by this, but real table also has this question. I have updated IMPALA-9356. Need I fix inline view by this?
http://gerrit.cloudera.org:8080/#/c/15047/11/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/15047/11/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1186
PS11, Line 1186: it does not evalute to
: // true with null
> "since it does not evaluate to true with null slots".
Done
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2339
PS7, Line 2339: ====
> It'd be better to add more test cases. E.g:
Done
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2368
PS9, Line 2368: d=b.id
> This looks strange to me. rand() returns a random value between 0 and 1 so
Yes, this is a bug. I reported https://issues.apache.org/jira/browse/IMPALA-9356
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2402
PS9, Line 2402: a
> nit: put the space after the comma
Done
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2442
PS9, Line 2442: FROM functional.a
> Should we move the function inside the view? This can be propagated without
Done
http://gerrit.cloudera.org:8080/#/c/15047/10/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/10/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2339
PS10, Line 2339: ====
> Why removing the test coverage for local views?
I deleted it by mistake, I will add it back
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1001
PS7, Line 1001: | | having: int_col = 17
> Could you clarify why the HAVING predicate got added..considering that it i
This is an existing problem, and I reported the improvement IMPALA-9305.
http://gerrit.cloudera.org:8080/#/c/15047/11/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/11/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1055
PS11, Line 1055: select a.id, b.id
> Can you add a similar test, except with b.int_col in the ON clause?
More tests in inline-view.test
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 19 Mar 2020 01:37:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is an improvement that tries to propagate predicates of the
nullable side of the outer join into inline view.
For example:
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val,
length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0
Before this change, the predicate b.len=0 can't be migrated into inline
view since that is on the nullable side of an outer join if the
predicate evaluates in the inline view nulls will not be rejected.
However, we can be more aggressive. In particular, some predicates that
must be evaluted at a join node can also be safely evaluted by the
outer-joined inline view. Such predicates are not marked as assigned.
The predicates propagate into the inline view and also be evaluated at
a join node.
We can divide predicates into two types. One that satisfies the condition
that same as Analyzer#canEvalPredicate can be migrated into inline view，
and one that satisfies the below three conditions is safe to be propagated
into the nullable side of an outer join.
1) The predicate needs to be bound by tupleIds.
2) The predicate is not on-clause.
3) The predicate evaluates to false when all its referenced tuples are NULL.
Therefore, 'b.upper_val is NULL' cannot be propagated to inline view but
‘b.len = 0’ can be propagated to inline view.
Tests:
* Add plan tests in inline-view.test
* One baseline plan in inline-view.test, one in nested-collections.test
and two in predicate-propagation.test had to be updated
* Ran the full set of verifications in Impala Public Jenkins
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
7 files changed, 317 insertions(+), 73 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/12
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 13: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 13
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 19 Mar 2020 07:17:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 12: Code-Review+2
(1 comment)
> Patch Set 12:
>
> (26 comments)
>
> I'm really sorry that because I'm not familiar with gerrit, I found that none of the replies were sent out. Thank you for reviewing my code. I apologize again for not replying before
No worry. Thanks for your patience on updating the patch!
Carry on Tim's +1. Thanks for your contribution, Xianqing!
http://gerrit.cloudera.org:8080/#/c/15047/10/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/15047/10/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1178
PS10, Line 1178: evalInInlineViewPreds.add(e);
> For inline view we can fix by this, but real table also has this question.
Let's fix this in IMPALA-9356.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 19 Mar 2020 02:15:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5526/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 26 Jan 2020 16:51:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 10:
(3 comments)
Took another round. The patch looks good to me in general.
Also add Tim since he knows some background of the planner.
http://gerrit.cloudera.org:8080/#/c/15047/10/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/15047/10/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1178
PS10, Line 1178: evalInInlineViewPreds.add(e);
Can we simply fix IMPALA-9356 by adding this to evalAfterJoinPreds?
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2368
PS9, Line 2368:
> This looks strange to me. rand() returns a random value between 0 and 1 so
I'm ok to skip adding this test as you did. I saw you create IMPALA-9356.
http://gerrit.cloudera.org:8080/#/c/15047/10/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/10/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2339
PS10, Line 2339: ====
Why removing the test coverage for local views?
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 10 Feb 2020 02:36:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 13
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 19 Mar 2020 02:16:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5447/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 11:36:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5346/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 17 Feb 2020 12:39:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 26 Jan 2020 23:02:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5353/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 07:46:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11:
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5352/
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 06:02:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5282/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 05 Feb 2020 01:48:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
IMPALA-8361: Fix bound predicates optimization doesn't work for
InlineView
This is improvement that migrated predicates of the nullable side of
the outer join into inline view.We should duplicate it and evaluate
after join.
For example:
EXPLAIN
SELECT count(*) FROM functional.alltypessmall a
LEFT JOIN
(SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny) b ON a.id=b.id
WHERE b.upper_val='1';
+------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------+
| Max Per-Host Resource Reservation: Memory=1.95MB Threads=5 |
| Per-Host Resource Estimates: Memory=86MB |
| Codegen disabled by planner |
| |
| PLAN-ROOT SINK |
| | |
| 06:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | row-size=8B cardinality=1 |
| | |
| 05:EXCHANGE [UNPARTITIONED] |
| | |
| 03:AGGREGATE |
| | output: count(*) |
| | row-size=8B cardinality=1 |
| | |
| 02:HASH JOIN [LEFT OUTER JOIN, BROADCAST] |
| | hash predicates: a.id = id |
| | other predicates: upper(string_col) = '1' <-eval after join
| | row-size=21B cardinality=100 |
| | |
| |--04:EXCHANGE [BROADCAST] |
| | | |
| | 01:SCAN HDFS [functional.alltypestiny] |
| | HDFS partitions=4/4 files=4 size=460B |
| | predicates: upper(string_col) = '1' <-push down |
| | row-size=17B cardinality=4 |
| | |
| 00:SCAN HDFS [functional.alltypessmall a] |
| HDFS partitions=4/4 files=4 size=6.32KB |
| row-size=4B cardinality=100 |
+------------------------------------------------------------+
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
6 files changed, 86 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/3
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 3
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5481/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 11:55:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5446/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 05:52:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15047
to look at the new patch set (#4).
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
IMPALA-8361: Fix bound predicates optimization doesn't work for
InlineView
This is improvement that migrated predicates of the nullable side of
the outer join into inline view.We should duplicate it and evaluate
after join.
For example:
EXPLAIN
SELECT count(*) FROM functional.alltypessmall a
LEFT JOIN
(SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny) b ON a.id=b.id
WHERE b.upper_val='1';
+------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------+
| Max Per-Host Resource Reservation: Memory=1.95MB Threads=5 |
| Per-Host Resource Estimates: Memory=86MB |
| Codegen disabled by planner |
| |
| PLAN-ROOT SINK |
| | |
| 06:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | row-size=8B cardinality=1 |
| | |
| 05:EXCHANGE [UNPARTITIONED] |
| | |
| 03:AGGREGATE |
| | output: count(*) |
| | row-size=8B cardinality=1 |
| | |
| 02:HASH JOIN [LEFT OUTER JOIN, BROADCAST] |
| | hash predicates: a.id = id |
| | other predicates: upper(string_col) = '1' <-eval after join
| | row-size=21B cardinality=100 |
| | |
| |--04:EXCHANGE [BROADCAST] |
| | | |
| | 01:SCAN HDFS [functional.alltypestiny] |
| | HDFS partitions=4/4 files=4 size=460B |
| | predicates: upper(string_col) = '1' <-push down |
| | row-size=17B cardinality=4 |
| | |
| 00:SCAN HDFS [functional.alltypessmall a] |
| HDFS partitions=4/4 files=4 size=6.32KB |
| row-size=4B cardinality=100 |
+------------------------------------------------------------+
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
6 files changed, 86 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/4
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 4
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 10: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5282/
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 05 Feb 2020 07:11:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 5:
(7 comments)
> (7 comments)
>
> Thanks for digging into this!
>
> If I understand this solution correctly, the key change of this
> patch is inside SingleNodePlanner#migrateConjunctsToInlineView().
> Before this change, we skip predicate b.upper_val='1' since
> canEvalPredicate() returns false on it. Because it doesn't satisfy
> isLastOjMaterializedByTupleIds(). So we can't migrate it inside the
> inline view. However, we can be more aggressive. It's correct to
> duplicate(not migrate) this predicate inside the inline view since
> it's not evaluted to true with null slots. This patch picks up the
> predicates that being skipped due to not satisfying
> isLastOjMaterializedByTupleIds() and duplicates them if they
> satisfy !isTrueWithNullSlots().
>
> I left some comments for refactoring. Hope we can make this part
> clearer.
I refactored this modification according to your comments, thanks!
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG@7
PS4, Line 7: Propagate predicates of outer-joined InlineView
:
> This is an improvement rather than a fix. I think this is better: "Propagat
Done
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG@10
PS4, Line 10: the outer join into inline view.
: I refactored the code "Analyzer#canEvalPredicate()" and add
: "SingleNode
> Not all the predicates of the nullable side could be duplicated into the in
Done
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1623
PS4, Line 1623: // Ignore 'tid' because it is not outer-joined.
: if (rhsRef == null) continue;
: // Check whether the last join to outer-join 'tid' is materialized by tupleIds.
: if (!tupleIds.containsAll(rhsRef.getAllTableRefIds())) return false;
: }
: return true;
: }
:
: public boolean canEvalOnClauseConjunct(List<TupleId> tupleIds, Expr e) {
: if (isAntiJoinedConjunct(e)) return canEvalAntiJoinedConjunct(e, tupleIds);
:
: List<TupleId> tids = new ArrayList<>();
: e.getIds(tids, null);
: if (tids.isEmpty()) return true;
:
: if (isIjConjunct(e) || isSjConjunct(e)) {
: if (!containsOuterJoinedTid(tids)) return true;
: // If the predicate references an outer-joined tuple, then evaluate it at
: // the join that the On-clause belongs to.
: TableRef onClauseTableRef = null;
: if (isIjConjunct(e)) {
: onClauseTableRef = globalState_.ijClauseByConjunct.get(e.getId());
: } else {
: onClauseTableRef = globalState_.sjClauseByConjunct.get(e.getId());
: }
: Preconditions.checkNotNull(onClauseTableRef);
: return tupleIds.containsAll(onClauseTableRef.getAllTableRefIds());
: }
:
: if (isFullOuterJoined(e)) return c
> The new overload of canEvalPredicate() is not quite clear in my opinion. We
Done
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@184
PS4, Line 184: conjuncts_ = orderConjunctsByCost(conjuncts_);
> Can we move this into MultiAggregateInfo#collectConjuncts()?
Done
http://gerrit.cloudera.org:8080/#/c/15047/4/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/15047/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1191
PS4, Line 1191: if (LOG. isTraceEnabled()) {
: LOG.trace(String.format("Can propagate %s to inline view %s",
: e.debugString(), inlineViewRef.getExplicitAlias()));
: }
: }
: continue;
: } catch (InternalException ex) {
: // Expr evaluation failed in the backend. Skip 'e' since we cannot
: // determine whether propagation is safe.
: LOG.warn("Skipping propagation of conjunct because backend evaluation failed: "
: + e.toSql(), ex);
: continue;
: }
: }
: if (LOG. isTraceEnabled()) {
: LOG.trace(String.format("Can evaluate %s in inline view %s", e.debugString(),
:
> Althouth the first if-clause is semantically equal to the old logics, it's
Done
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1219
PS4, Line 1219: * makes the *output* of the computation visible to the enclosing scope, so that
: * filters from the enclosing scope can be safe
> "Propagate the conjuncts evaluating the nullable side of outer-join. Don't
Done
http://gerrit.cloudera.org:8080/#/c/15047/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1002
PS4, Line 1002: 73
> This is actually 73 after adding the predicate. Note that the test passes s
Done
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 05:50:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is improvement that propagate predicates of the nullable side of
the outer join into inline view.
I refactored the code "Analyzer#canEvalPredicate()" and add
"SingleNodePlanner#getConjunctsToInlineView()" to get conjucts can be
correctly to migrate or propagate into inline view.
Before this change, we skip predicate b.upper_val='1' since
canEvalPredicate() returns false on it. Because it doesn't satisfy
isLastOjMaterializedByTupleIds(). So we can't migrate it inside the
inline view. However, we can be more aggressive. It's correct to
duplicate(not migrate) this predicate inside the inline view since
it's not evaluted to true with null slots.
This patch picks up the predicates that being skipped due to not
satisfying isLastOjMaterializedByTupleIds() and duplicates them
if they satisfy !isTrueWithNullSlots().
For example:
EXPLAIN
SELECT count(*) FROM functional.alltypessmall a
LEFT JOIN
(SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny) b ON a.id=b.id
WHERE b.upper_val='1';
+------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------+
| Max Per-Host Resource Reservation: Memory=1.95MB Threads=5 |
| Per-Host Resource Estimates: Memory=86MB |
| Codegen disabled by planner |
| |
| PLAN-ROOT SINK |
| | |
| 06:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | row-size=8B cardinality=1 |
| | |
| 05:EXCHANGE [UNPARTITIONED] |
| | |
| 03:AGGREGATE |
| | output: count(*) |
| | row-size=8B cardinality=1 |
| | |
| 02:HASH JOIN [LEFT OUTER JOIN, BROADCAST] |
| | hash predicates: a.id = id |
| | other predicates: upper(string_col) = '1' <-eval after join
| | row-size=21B cardinality=100 |
| | |
| |--04:EXCHANGE [BROADCAST] |
| | | |
| | 01:SCAN HDFS [functional.alltypestiny] |
| | HDFS partitions=4/4 files=4 size=460B |
| | predicates: upper(string_col) = '1' <-push down |
| | row-size=17B cardinality=4 |
| | |
| 00:SCAN HDFS [functional.alltypessmall a] |
| HDFS partitions=4/4 files=4 size=6.32KB |
| row-size=4B cardinality=100 |
+------------------------------------------------------------+
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
6 files changed, 146 insertions(+), 57 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/5
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5483/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 12:47:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5503/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 13:26:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: ,
nit: need space
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: the predicates that
: must be evaluted at a join node but can also be safely evaluted by the
: outer-joined inline view.
I think you mean "some predicates that must be evaluted at a join node can also be safely evaluted by the outer-joined inline view".
http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@30
PS9, Line 30: .
nit: need space
http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@985
PS9, Line 985: picked up by getBoundPredicates()
I think this comment is stale after merging this patch. Could you update it to "picked up by getBoundPredicates() and migrateConjunctsToInlineView()"?
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2368
PS9, Line 2368: predicates: rand() = 12
This looks strange to me. rand() returns a random value between 0 and 1 so "rand() = 12" will always be false. All rows should be rejected by the WHERE clause. If "rand() = 12" is evaluated in only one side, the other side can still produce rows. So the outer join will still have results.
However, looks like the original planner has the same plan. Could you create a JIRA for this? I think it's a bug. It's worth to mention it in the above comments.
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2402
PS9, Line 2402: ,
nit: put the space after the comma
http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2442
PS9, Line 2442: upper(b.string_col)
Should we move the function inside the view? This can be propagated without this patch. Maybe change it to
SELECT * FROM functional.alltypestiny a
LEFT JOIN
(SELECT upper(b.string_col) as string_col, b.id FROM functional.alltypestiny a LEFT JOIN
functional.alltypestiny b ON a.id=b.id) b ON a.id=b.id
WHERE b.string_col='1';
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 04 Feb 2020 03:49:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5475/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 06:32:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 05 Feb 2020 12:51:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5465/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 26 Jan 2020 18:17:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5425/
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jan 2020 08:19:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is an improvement that tries to propagate predicates of the
nullable side of the outer join into inline view.
For example:
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val,
length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0
Before this change, the predicate b.len=0 can't be migrated into inline
view since that is on the nullable side of an outer join if the
predicate evaluates in the inline view nulls will not be rejected.
However, we can be more aggressive. In particular, some predicates that
must be evaluted at a join node can also be safely evaluted by the
outer-joined inline view. Such predicates are not marked as assigned.
The predicates propagate into the inline view and also be evaluated at
a join node.
We can divide predicates into two types. One that satisfies the condition
that same as Analyzer#canEvalPredicate can be migrated into inline view，
and one that satisfies the below three conditions is safe to be propagated
into the nullable side of an outer join.
1) The predicate needs to be bound by tupleIds.
2) The predicate is not on-clause.
3) The predicate evaluates to false when all its referenced tuples are NULL.
Therefore, 'b.upper_val is NULL' cannot be propagated to inline view but
‘b.len = 0’ can be propagated to inline view.
Tests:
* Add plan tests in inline-view.test
* One baseline plan in inline-view.test, one in nested-collections.test
and two in predicate-propagation.test had to be updated
* Ran the full set of verifications in Impala Public Jenkins
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
7 files changed, 281 insertions(+), 58 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/10
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5442/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 4
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 20 Jan 2020 02:31:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "xqhe (Code Review)" <ge...@cloudera.org>.
xqhe has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2:
> hexianqing, thanks for your contribution. I will review this over
> the weekend. BTW, there are PlannerTest failures according to the
> jenkins run.
Because of this optimization,some plan of the PlannerTest will change, I will fix it soon, thank you.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: xqhe <he...@126.com>
Gerrit-Comment-Date: Sat, 18 Jan 2020 14:50:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 7:
(11 comments)
The patchs LGTM. Left some comments on code readability and tests.
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@9
PS7, Line 9: This is improvement that propagate
"This is an improvement that tries to propagate"
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@11
PS7, Line 11: I refactored the code "Analyzer#canEvalPredicate()" and add
: "SingleNodePlanner#getConjunctsToInlineView()" to get conjucts can be
: correctly to migrate or propagate into inline view.
: Before this change, we skip predicate b.upper_val='1' since
: canEvalPredicate() returns false on it. Because it doesn't satisfy
: isLastOjMaterializedByTupleIds(). So we can't migrate it inside the
: inline view. However, we can be more aggressive. It's correct to
: duplicate(not migrate) this predicate inside the inline view since
: it's not evaluted to true with null slots.
Please take some time to write the commit message. It's very important for others to understand your work. Your commit message will be read many times so it worth your time to polish it. It should contain what the problem was, and how it was fixed. Talking too much about codes doesn't help for understanding it. We have some guidelines in the wiki: https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala
Here are some examples for planner fixes:
https://github.com/apache/impala/commit/df5c4061456abb947cec8add81b361b60c5d3ad8
https://github.com/apache/impala/commit/c665fc1e06d53c7b70611ad3993c712c7fe35cb2
https://github.com/apache/impala/commit/ae8295118191486f31da4d8d3c9d0f7e7e5d4b3a
https://github.com/apache/impala/commit/f25a899924856f705ecb581b237a149003279473
https://github.com/apache/impala/commit/5e9b4e2fd2a459a3b61e8939e3085ad8e1a5795e
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1630
PS7, Line 1630:
Please add a function comment like other "canEval*" functions.
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1631
PS7, Line 1631: public boolean canEvalOnClauseConjunct(List<TupleId> tupleIds, Expr e) {
Add Preconditions.checkState(e.isOnClauseConjunct()) at the beginning of this function.
http://gerrit.cloudera.org:8080/#/c/15047/7/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/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1162
PS7, Line 1162: Get the conjuncts evaluating in inline view.
"Get the unassigned conjuncts that can be evaluated in inline view and return them in 'preds'."
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1163
PS7, Line 1163: * The unassigned conjuncts can be correctly evaluated by inline view, add them to preds
: * and if propagation is safe, add them to evalAfterJoinPreds.
If a conjunct is not an On-clause predicate and is safe to propagate it inside the inline view, add it to 'evalAfterJoinPreds'.
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1167
PS7, Line 1167: preds
I think something like "evalInViewPreds" may be more clear for its meaning.
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1176
PS7, Line 1176: if (tids.isEmpty()) {
: preds.add(e);
Just be curious, can we add test coverage for this case?
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1190
PS7, Line 1190:
nit: remove space here
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1203
PS7, Line 1203:
nit: remove space here
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2339
PS7, Line 2339: ====
It'd be better to add more test cases. E.g:
# Local view
WITH b as (SELECT id, upper(string_col) AS upper_val FROM functional.alltypestiny)
SELECT * FROM functional.alltypessmall a LEFT JOIN b ON a.id = b.id
WHERE b.upper_val = '1';
# Where-clause predicate that can't be propagated
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL;
# More predicates in where-clause
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id + 1 as id, upper(string_col) AS upper_val, length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0 and b.id > 0;
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 08:41:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 21 Jan 2020 10:40:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/5443/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 2
Gerrit-Owner: xqhe <he...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2020 13:42:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 4:
(7 comments)
Thanks for digging into this!
If I understand this solution correctly, the key change of this patch is inside SingleNodePlanner#migrateConjunctsToInlineView(). Before this change, we skip predicate b.upper_val='1' since canEvalPredicate() returns false on it. Because it doesn't satisfy isLastOjMaterializedByTupleIds(). So we can't migrate it inside the inline view. However, we can be more aggressive. It's correct to duplicate(not migrate) this predicate inside the inline view since it's not evaluted to true with null slots. This patch picks up the predicates that being skipped due to not satisfying isLastOjMaterializedByTupleIds() and duplicates them if they satisfy !isTrueWithNullSlots().
I left some comments for refactoring. Hope we can make this part clearer.
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG@7
PS4, Line 7: Fix bound predicates optimization doesn't work for
: InlineView
This is an improvement rather than a fix. I think this is better: "Propagate predicates of outer-joined InlineView".
http://gerrit.cloudera.org:8080/#/c/15047/4//COMMIT_MSG@10
PS4, Line 10: This is improvement that migrated predicates of the nullable side of
: the outer join into inline view.We should duplicate it and evaluate
: after join.
Not all the predicates of the nullable side could be duplicated into the inline view, e.g. "b.upper_val is null" can only be evaluated at the JOIN node. Hope you can explain your change in more details.
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1623
PS4, Line 1623: if (isAntiJoinedConjunct(e)) return canEvalAntiJoinedConjunct(e, tupleIds);
:
: if (isIjConjunct(e) || isSjConjunct(e)) {
: if (!containsOuterJoinedTid(tids)) return true;
: // If the predicate references an outer-joined tuple, then evaluate it at
: // the join that the On-clause belongs to.
: TableRef onClauseTableRef = null;
: if (isIjConjunct(e)) {
: onClauseTableRef = globalState_.ijClauseByConjunct.get(e.getId());
: } else {
: onClauseTableRef = globalState_.sjClauseByConjunct.get(e.getId());
: }
: Preconditions.checkNotNull(onClauseTableRef);
: return tupleIds.containsAll(onClauseTableRef.getAllTableRefIds());
: }
:
: if (isFullOuterJoined(e)) return canEvalFullOuterJoinedConjunct(e, tupleIds);
: if (isOjConjunct(e)) {
: // Force this predicate to be evaluated by the corresponding outer join node.
: // The join node will pick up the predicate later via getUnassignedOjConjuncts().
: if (tids.size() > 1) return false;
: // Optimization for single-tid predicates: Legal to assign below the outer join
: // if the predicate is from the same On-clause that makes tid nullable
: // (otherwise e needn't be true when that tuple is set).
: TupleId tid = tids.get(0);
: return globalState_.ojClauseByConjunct.get(e.getId()) == getLastOjClause(tid);
: }
:
: // Should have returned in one of the cases above.
: Preconditions.checkState(false);
The new overload of canEvalPredicate() is not quite clear in my opinion. We can encapsulate these into a function 'canEvalOnClauseConjunct(tupleIds, e)' and use it directly in SingleNodePlanner#migrateConjunctsToInlineView().
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@184
PS4, Line 184: Expr.removeDuplicates(conjuncts_);
Can we move this into MultiAggregateInfo#collectConjuncts()?
http://gerrit.cloudera.org:8080/#/c/15047/4/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/15047/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1191
PS4, Line 1191: if (analyzer.canEvalPredicate(tupleIds, e, false)) {
: if (e.isOnClauseConjunct() ||
: analyzer.isLastOjMaterializedByTupleIds(tupleIds, e)) {
: preds.add(e);
: } else {
: // Deal predicate belong to an outer-joined tuple
: try {
: if (analyzer.isTrueWithNullSlots(e)) continue;
: } catch (InternalException ex) {
: // Expr evaluation failed in the backend. Skip 'e' since we cannot
: // determine whether propagation is safe.
: LOG.warn("Skipping propagation of conjunct because backend evaluation "
: + "failed: " + e.toSql(), ex);
: continue;
: }
: evalAfterJoinPreds.add(e);
: }
Althouth the first if-clause is semantically equal to the old logics, it's not quite clear since e.isOnClauseConjunct() and analyzer.isLastOjMaterializedByTupleIds(tupleIds, e) which are part of canEvalPredicate(). I recommend refactoring it to not invoke canEvalPredicate() like this (with my comment in Analyzer.java):
if (e.isOnClauseConjunct()) {
if (analyzer.canEvalOnClauseConjunct(tupleIds, e)) preds.add(e);
} else if (analyzer.isLastOjMaterializedByTupleIds(tupleIds, e)) {
preds.add(e);
} else {
// Deal with other predicates bounded to an outer-joined tuple
try {
if (analyzer.isTrueWithNullSlots(e)) continue;
} catch (InternalException ex) {
// Expr evaluation failed in the backend. Skip 'e' since we cannot
// determine whether propagation is safe.
LOG.warn("Skipping propagation of conjunct because backend evaluation "
+ "failed: " + e.toSql(), ex);
continue;
}
evalAfterJoinPreds.add(e);
}
Also add some trace logging for better debugging. What do you think?
http://gerrit.cloudera.org:8080/#/c/15047/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1219
PS4, Line 1219: // Migrate the conjucts evaluate the nullable side of outer-join. They should also
: // evaluate after join, not mark as assigned.
"Propagate the conjuncts evaluating the nullable side of outer-join. Don't mark them as assigned so they would be assigned at the JOIN node."
http://gerrit.cloudera.org:8080/#/c/15047/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1002
PS4, Line 1002: 730
This is actually 73 after adding the predicate. Note that the test passes since it doesn't verify the cardinality (run without PlannerTestOption.VALIDATE_CARDINALITY). But it's good to correct the value as well.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 4
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 20 Jan 2020 14:46:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5238/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 17 Feb 2020 13:14:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11: Code-Review+1
(5 comments)
Thank you for the patch! The optimisation makes sense to me. I have some minor comments about comments and tests, but otherwise looks good to me.
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1588
PS11, Line 1588: * - For On-clause predicates:
I think this part of the comment about on-clause predicates should be moved to the commend of "canEvalOnClauseConjunct"
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1643
PS11, Line 1643: List<TupleId> tids = new ArrayList<>();
Maybe rename this to exprTids or similar, so that it's clearer how it is different from tupleIds.
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/15047/11/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@617
PS11, Line 617: * returned by getResultTupleId().
Document that it only returns unique expressions?
http://gerrit.cloudera.org:8080/#/c/15047/11/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/15047/11/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1186
PS11, Line 1186: it's not evaluted to true
: // with null slots
"since it does not evaluate to true with null slots".
http://gerrit.cloudera.org:8080/#/c/15047/11/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/11/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1055
PS11, Line 1055: select a.id, b.id
Can you add a similar test, except with b.int_col in the ON clause?
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 09 Mar 2020 22:40:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 12
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 17:34:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Xianqing He (Code Review)" <ge...@cloudera.org>.
Xianqing He has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
IMPALA-8361: Propagate predicates of outer-joined InlineView
This is an improvement that tries to propagate predicates of the
nullable side of the outer join into inline view.
For example:
SELECT *
FROM functional.alltypessmall a
LEFT JOIN (
SELECT id, upper(string_col) AS upper_val,
length(string_col) AS len
FROM functional.alltypestiny
) b ON a.id = b.id
WHERE b.upper_val is NULL and b.len = 0
Before this change, the predicate b.len=0 can't be migrated into inline
view since that is on the nullable side of an outer join if the
predicate evaluates in the inline view nulls will not be rejected.
However, we can be more aggressive. In particular, some predicates that
must be evaluted at a join node can also be safely evaluted by the
outer-joined inline view. Such predicates are not marked as assigned.
The predicates propagate into the inline view and also be evaluated at
a join node.
We can divide predicates into two types. One that satisfies the condition
that same as Analyzer#canEvalPredicate can be migrated into inline view，
and one that satisfies the below three conditions is safe to be propagated
into the nullable side of an outer join.
1) The predicate needs to be bound by tupleIds.
2) The predicate is not on-clause.
3) The predicate evaluates to false when all its referenced tuples are NULL.
Therefore, 'b.upper_val is NULL' cannot be propagated to inline view but
‘b.len = 0’ can be propagated to inline view.
Tests:
* Add plan tests in inline-view.test
* One baseline plan in inline-view.test, one in nested-collections.test
and two in predicate-propagation.test had to be updated
* Ran the full set of verifications in Impala Public Jenkins
Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
7 files changed, 303 insertions(+), 58 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/15047/11
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5468/ : 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/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 3
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 19 Jan 2020 12:33:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15047/7//COMMIT_MSG@11
PS7, Line 11: I refactored the code "Analyzer#canEvalPredicate()" and add
: "SingleNodePlanner#getConjunctsToInlineView()" to get conjucts can be
: correctly to migrate or propagate into inline view.
: Before this change, we skip predicate b.upper_val='1' since
: canEvalPredicate() returns false on it. Because it doesn't satisfy
: isLastOjMaterializedByTupleIds(). So we can't migrate it inside the
: inline view. However, we can be more aggressive. It's correct to
: duplicate(not migrate) this predicate inside the inline view since
: it's not evaluted to true with null slots.
> Please take some time to write the commit message. It's very important for
Agree with Quanlong regarding improving the commit message. In addition, I would suggest NOT putting the entire Explain plan in the commit message. These can be quite verbose and is best described in the JIRA.
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15047/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1618
PS7, Line 1618: public boolean isLastOjMaterializedByTupleIds(List<TupleId> tupleIds, Expr e) {
Pls add javadoc comment
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test:
http://gerrit.cloudera.org:8080/#/c/15047/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1001
PS7, Line 1001: | | having: int_col = 17
Could you clarify why the HAVING predicate got added..considering that it is already pushed to the Scan node. This predicate looks redundant.
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 7
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 23 Jan 2020 07:20:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................
Patch Set 11: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5346/
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 11
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 17 Feb 2020 16:58:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )
Change subject: IMPALA-8361: Fix bound predicates optimization doesn't work for InlineView
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 4
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 20 Jan 2020 07:16:42 +0000
Gerrit-HasComments: No