You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gabor Kaszab (Code Review)" <ge...@cloudera.org> on 2021/12/13 16:22:51 UTC

[Impala-ASF-CR] WiP: Zipping unnest from view

Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18094


Change subject: WiP: Zipping unnest from view
......................................................................

WiP: Zipping unnest from view

Based on PS26 of the array patch.

I'd consider the part with the select list syntax ready, while the from
clause syntax has some issues with the predicates.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
A testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
M tests/query_test/test_nested_types.py
20 files changed, 749 insertions(+), 60 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18094/6/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/18094/6/common/thrift/PlanNodes.thrift@657
PS6, Line 657:   2: required i32 output_tuple_id
Do I understand correctly that until now we were using the id of the item tuple desc of the collection_expr, but now the the id of this tuple can be different from the output tuple, while their layout will be the same, and no materialization actually occurs?


http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@120
PS6, Line 120:       // A view has already registered this TableRef. Let's find this view and do some
It is registered here, right? https://github.com/apache/impala/blob/d3da875684598f0d734fb78a83751b72c00e9a02/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java#L281

Won't we also set sourceView in this case?
https://github.com/apache/impala/blob/d3da875684598f0d734fb78a83751b72c00e9a02/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java#L235


http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@128
PS6, Line 128:       processParentTableRefForZippingUnnest((InlineViewRef)parentTableRef);
Is this really different to 
collectionExpr_ =
  collectionExpr_.trySubstitute(sourceView.getBaseTblSmap(), analyzer, true);
at line 156


http://gerrit.cloudera.org:8080/#/c/18094/6/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/18094/6/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@521
PS6, Line 521: getExprSubsMapFromTableRef
The name is a bit misleading, as it returns the baseTblSmap.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Mar 2022 16:16:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#3).

Change subject: WiP: Zipping unnest from view
......................................................................

WiP: Zipping unnest from view

Based on PS26 of the array patch.

I'd consider the part with the select list syntax ready, while the from
clause syntax has some issues with the predicates.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
A testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
M tests/query_test/test_nested_types.py
20 files changed, 765 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#4).

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch inproves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes to cover when the source of
   the unnested arrays is not a table but a view. Also tested
   multi-level views and filtering the unnested items on different
   levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
20 files changed, 1,032 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10272/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Mar 2022 13:18:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: WiP: Zipping unnest from view
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9921/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 09:44:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: WiP: Zipping unnest from view
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18094/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@888
PS2, Line 888:       if (analyzer.getNumZippingUnnests() > 1 && zippingUnnestTupleIds.contains(itemTid)) {
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 09:21:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 4:

(13 comments)

Only some nits, though I haven't gone through the tests yet.
Thanks.

http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG@10
PS4, Line 10: n
Nit: improves.


http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG@16
PS4, Line 16: both unnesting syntaxes
Nit: we could write the the possibilities in parentheses: select list and from clause. It may make it easier to understand it for someone that doesn't know the context well.


http://gerrit.cloudera.org:8080/#/c/18094/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/18094/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@712
PS4, Line 712: it
Could you make it clear that the returned TableRef is not the one passed in but the one corresponding to it that has been registered? Or are they the same?


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@221
PS4, Line 221: multipl
Nit: typo.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java@149
PS4, Line 149:     // Don't do any checks of the collection that came from a view as getTable() would
             :     // return null in that case.
Does this comment refer to L151 or L152?


http://gerrit.cloudera.org:8080/#/c/18094/4/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/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@467
PS4, Line 467:       if (analyzer_.getNumZippingUnnests() <= 1) return;
Nit: this condition can be checked before getting the zipping unnest tuples (on the previous line).


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@472
PS4, Line 472: Lists.newArrayList
We could convert 'zippingUnnestTupleIds' to a List already on L466 so we don't have to do it in a loop.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@485
PS4, Line 485: functions identifie
Nit: function identifies.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@506
PS4, Line 506: is
Nit: if.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
File fe/src/main/java/org/apache/impala/analysis/SlotRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@378
PS4, Line 378: _
Nit: space after underscore.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
File fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java@43
PS4, Line 43: This way "unnest(array_name)" and "unnest(array_name.item)" would
            :     // both work.
Is this needed or good? When (or if) we add support for nested arrays, this could lead to confusion as to whether the outer or the inner array is unnested.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java@514
PS4, Line 514: unnestExprs
Isn't the name 'unnestExprs' a bit misleading? As far as understand they are only really unnest expressions if 'zippingUnnestTupleIds.contains()' is true for them.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/UnnestNode.java
File fe/src/main/java/org/apache/impala/planner/UnnestNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/UnnestNode.java@128
PS4, Line 128: ToSqlUtils.getPathSql(t2.getPath()
Nit: if path1 is extracted to a variable, so could also "path2".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 17:17:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9: Code-Review+2

Carry +2 from Csaba


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Apr 2022 07:56:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#9).

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch improves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes (select list and from clause)
   to cover when the source of the unnested arrays is not a table but a
   view. Also tested multi-level views and filtering the unnested items
   on different levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
19 files changed, 945 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 17:32:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: WiP: Zipping unnest from view
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9916/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Dec 2021 16:45:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10327/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Mar 2022 15:58:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10359/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Mar 2022 09:30:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch improves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes (select list and from clause)
   to cover when the source of the unnested arrays is not a table but a
   view. Also tested multi-level views and filtering the unnested items
   on different levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Reviewed-on: http://gerrit.cloudera.org:8080/18094
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Gabor Kaszab <ga...@cloudera.com>
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
19 files changed, 945 insertions(+), 49 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Gabor Kaszab: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 10
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9:

PS9 is rebase with master


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 12:47:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 8: Code-Review+2

(3 comments)

I still think that there are possibilities to simplify this patch, but it is probably better to work on these in another change once https://gerrit.cloudera.org/#/c/17847/ is also merged.

http://gerrit.cloudera.org:8080/#/c/18094/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/18094/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@510
PS7, Line 510:  If the table ref is originated from a view then also add the tuple IDs for the
             :     // respective table refs from the view.
> I think this is still true as e.g. for a from clause unnest I still had to 
Oops, I have missed some of the callsites.


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@206
PS7, Line 206: isCollectionInSelectList
> I use it twice in UnnestExpr.
oops, why did I miss that :/


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java@150
PS7, Line 150:     // Don't do any checks of the collection that came from a view as getTable() would
             :     // return null in that case.
             :     if (collRef.getTable() == null) return;
> removing this check would crash this query:
In the example query the arrays do not come from a view, while the comment says that it is related to collectionTableRefs from views.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 08:05:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9:

carry +2 from Csaba


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 13:02:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#7).

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch improves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes (select list and from clause)
   to cover when the source of the unnested arrays is not a table but a
   view. Also tested multi-level views and filtering the unnested items
   on different levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
19 files changed, 943 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: WiP: Zipping unnest from view
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18094/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@888
PS1, Line 888:       if (analyzer.getNumZippingUnnests() > 1 && zippingUnnestTupleIds.contains(itemTid)) {
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Dec 2021 16:24:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 6:

(14 comments)

PS6 is a rebase with master

http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG@10
PS4, Line 10: m
> Nit: improves.
Done


http://gerrit.cloudera.org:8080/#/c/18094/4//COMMIT_MSG@16
PS4, Line 16: both unnesting syntaxes
> Nit: we could write the the possibilities in parentheses: select list and f
Done


http://gerrit.cloudera.org:8080/#/c/18094/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/18094/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@712
PS4, Line 712: it
> Could you make it clear that the returned TableRef is not the one passed in
Here actually the uniqueAlias is needed from the parameter TableRef. So I changed the function to receive an alias and re-wrote the comment as well.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@221
PS4, Line 221: multipl
> Nit: typo.
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java@149
PS4, Line 149:     if (collRef.getCollectionExpr() != null) return;
             :     // Don't do any checks of th
> Does this comment refer to L151 or L152?
L152. Done


http://gerrit.cloudera.org:8080/#/c/18094/4/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/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@467
PS4, Line 467:       List<TupleId> zippingUnnestTupleIds = Lists.newArrayList(
> Nit: this condition can be checked before getting the zipping unnest tuples
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@472
PS4, Line 472: nWhereClause) {
> We could convert 'zippingUnnestTupleIds' to a List already on L466 so we do
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@485
PS4, Line 485: fic for zipping unn
> Nit: function identifies.
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@506
PS4, Line 506: ol
> Nit: if.
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
File fe/src/main/java/org/apache/impala/analysis/SlotRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@378
PS4, Line 378: _
> Nit: space after underscore.
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
File fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java@43
PS4, Line 43:  adding "item" to the path in the first round of analysis here in the
            :     // unnest.
> Is this needed or good? When (or if) we add support for nested arrays, this
Well, that is some unintentionally left over comment and not valid anymore. Rephrased it.


http://gerrit.cloudera.org:8080/#/c/18094/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@888
PS2, Line 888:       if (!checkTypeForOverlapPredicate(slotRefInScan.getType(), targetExpr.getType())
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java@514
PS4, Line 514: slotRefs);
> Isn't the name 'unnestExprs' a bit misleading? As far as understand they ar
Indeed. renamed it.


http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/UnnestNode.java
File fe/src/main/java/org/apache/impala/planner/UnnestNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/4/fe/src/main/java/org/apache/impala/planner/UnnestNode.java@128
PS4, Line 128: ls.getPathSql(t2.getPath());
> Nit: if path1 is extracted to a variable, so could also "path2".
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 11 Mar 2022 09:00:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10245/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Mar 2022 13:14:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#5).

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch improves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes (select list and from clause)
   to cover when the source of the unnested arrays is not a table but a
   view. Also tested multi-level views and filtering the unnested items
   on different levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
20 files changed, 1,035 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18094/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/18094/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@510
PS7, Line 510:  If the table ref is originated from a view then also add the tuple IDs for the
             :     // respective table refs from the view.
Is this still true in the last patch?


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@515
PS7, Line 515:     public int numZippingUnnests = 0;
Do we still need this? I think that zippingUnnestTupleIds's length should be the same.


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@206
PS7, Line 206: isCollectionInSelectList
I couldn't find where we use this.
I think it remained by accident in my array patch:
https://github.com/apache/impala/blob/d3da875684598f0d734fb78a83751b72c00e9a02/fe/src/main/java/org/apache/impala/analysis/TableRef.java#L317


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java@150
PS7, Line 150:     // Don't do any checks of the collection that came from a view as getTable() would
             :     // return null in that case.
             :     if (collRef.getTable() == null) return;
Shouldn't line 149 be enough to rule out


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/planner/PlanNode.java@506
PS7, Line 506: removeZippingUnnestConjuncts
Is this still needed? What I don't get is why is it needed now, but why it was not needed when we didn't support views.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 08:50:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#2).

Change subject: WiP: Zipping unnest from view
......................................................................

WiP: Zipping unnest from view

Based on PS26 of the array patch.

I'd consider the part with the select list syntax ready, while the from
clause syntax has some issues with the predicates.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
A testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
M tests/query_test/test_nested_types.py
20 files changed, 764 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 7:

(4 comments)

Thanks for the comments and the effort to understand the code changes, Csaba! As you suggested, I managed to get rid of some over-complicated or duplicated part of the code.

http://gerrit.cloudera.org:8080/#/c/18094/6/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/18094/6/common/thrift/PlanNodes.thrift@657
PS6, Line 657: struct TCardinalityCheckNode {
> Do I understand correctly that until now we were using the id of the item t
Nevermind, it turned out that adding this extra field is not needed at all. I also could revert the related changes from the UnnestNode both in FE and BE.


http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@120
PS6, Line 120:     if (!isRelative() && resolvedPath_.getRootTable() instanceof FeView) {
> It is registered here, right? https://github.com/apache/impala/blob/d3da875
Indeed, "sourceView != null" kind of equals to "existingTableRef != null". But anyway I managed to get rid of this part of the code and rely on the logic you wrote below to substitute the exprs.


http://gerrit.cloudera.org:8080/#/c/18094/6/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@128
PS6, Line 128:         // existing tuple desc created by the view. This is not needed when
> Is this really different to 
You're right. See comment above, I dropped this part of the code.


http://gerrit.cloudera.org:8080/#/c/18094/6/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/18094/6/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@521
PS6, Line 521: getBaseTableSMapFromTableR
> The name is a bit misleading, as it returns the baseTblSmap.
Renamed it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Mar 2022 15:45:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WiP: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: WiP: Zipping unnest from view
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9922/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 09:51:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18094/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/18094/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@510
PS7, Line 510:  If the table ref is originated from a view then also add the tuple IDs for the
             :     // respective table refs from the view.
> Is this still true in the last patch?
I think this is still true as e.g. for a from clause unnest I still had to store the zipping unnest IDs both in the FromClause and in CollectionTableRef.analyze().
For the select list syntax again we store the zipping unnest IDs in the FromClause (in the re-analysis phase) and in UnnestExpr.analyze().

See the calsites for addZippingUnnestTupleId()


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@515
PS7, Line 515:     public int numZippingUnnests = 0;
> Do we still need this? I think that zippingUnnestTupleIds's length should b
see comment above.


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
File fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java@206
PS7, Line 206: isCollectionInSelectList
> I couldn't find where we use this.
I use it twice in UnnestExpr.


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/analysis/FromClause.java@150
PS7, Line 150:     // Don't do any checks of the collection that came from a view as getTable() would
             :     // return null in that case.
             :     if (collRef.getTable() == null) return;
> Shouldn't line 149 be enough to rule out
removing this check would crash this query:
select id, a1, a2
from (
    select id, unnest(arr1) a1, unnest(arr2) a2
    from complextypes_arrays
    where id = 3 or id = 4) x
where a1 > 8 and a2 = 'ten';


http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:

http://gerrit.cloudera.org:8080/#/c/18094/7/fe/src/main/java/org/apache/impala/planner/PlanNode.java@506
PS7, Line 506: removeZippingUnnestConjuncts
> Is this still needed? What I don't get is why is it needed now, but why it 
hmm, it was a long time ago when I wrote this ... :D
I did some poking here and it seems that for some reason the SingularRowSrcNode picks up zipping unnest conjuncts (and fails on a Precondition as it shouldn't) instead of the UnnestNode.
Also, if I remove this function the "select list unnest" tests also break.

Currently, I don't recall why this is needed for unnesting from views but not for tables. I'll take a look to figure out.

Update: Ok, I figured it out why this is needed. In case there are conjuncts on an unnested array where the array is coming from a view the UnnestNode originally (and none of the other nodes) didn't pick up these conjuncts and they weren't evaluated. I had to make the changes on isBoundByTupleIds() functions in UnnestExpr and SlotRef but then the ScanNode also wanted to pick up these, also the SingularRowSrcNode so this is what I came up with to prevent them to pick up these unnested array related conjuncts and let UnnestNode to take care of them.
The above only applies if there are more than 1 of these unnest conjuncts. If there is one, than ScanNode handles it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Mar 2022 09:15:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18094

to look at the new patch set (#8).

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................

IMPALA-11038: Zipping unnest from view

IMPALA-10920 introduced zipping unnest functionality for arrays that
are in a table. This patch improves that support further by accepting
inputs from views as well.

Testing:
 - Added planner tests to verify which execution node handles the
   predicates on unnested items.
 - E2E tests for both unnesting syntaxes (select list and from clause)
   to cover when the source of the unnested arrays is not a table but a
   view. Also tested multi-level views and filtering the unnested items
   on different levels.

Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
---
M be/src/exec/subplan-node.cc
M be/src/exec/unnest-node.cc
M be/src/exec/unnest-node.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/UnnestExpr.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/zipping-unnest.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-from-view.test
M testdata/workloads/functional-query/queries/QueryTest/zipping-unnest-in-select-list.test
19 files changed, 942 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/18094/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10391/ : 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/18094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 13:03:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18094 )

Change subject: IMPALA-11038: Zipping unnest from view
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8017/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68f649dda9e41f257e7f6596193d07b24049f92a
Gerrit-Change-Number: 18094
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 13:04:12 +0000
Gerrit-HasComments: No