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 <gaborkas...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Wed, 23 Mar 2022 15:45:28 +0000 Gerrit-HasComments: Yes
