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 <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 29 Mar 2022 08:50:58 +0000 Gerrit-HasComments: Yes
