[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 03 Mar 2022 13:14:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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: I68f649dda9e41f257e7f659
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 09 Mar 2022 13:18:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 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
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 22 Mar 2022 16:16:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 23 Mar 2022 15:45:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 23 Mar 2022 15:58:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 29 Mar 2022 08:50:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 30 Mar 2022 09:15:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 30 Mar 2022 09:30:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 08:05:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 12:47:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 13:03:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 13:04:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 13:02:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 04 Apr 2022 17:32:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Apr 2022 07:56:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11038: Zipping unnest from view
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 Reviewed-by: Gabor Kaszab --- 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 Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins