Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17847 )
Change subject: IMPALA-10838: Fix substitution and improve unification of struct slots ...................................................................... Patch Set 25: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/17847/23/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/17847/23/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@343 PS23, Line 343: if (whereClause_ != null) { : exprs = Stream.concat(exprs, Stream.of(whereClause_)); : } : : if (havingClause_ != null) { : exprs = Stream.concat(exprs, Stream.of(havingClause_)); : } : : if (groupingExprs_ != null) { : exprs = Stream.concat(exprs, groupingExprs_.stream()); : } : : if (sortInfo_ != null) { : exprs = Stream.concat(exprs, sortInfo_.getSortExprs().stream()); : } : : if (analyticInfo_ != null) { : exprs = Stream.concat(exprs, : analyticInfo_.getAnalyticExprs().stream() : .map(analyticExpr -> (Expr) analyticExpr)); : } > I wouldn't separate the collection of exprs in the select list and outside The reason why I would separate the select list is the star handling - we skipped it here, but this is only correct because star excludes complex types at the moment. If there will be a query option about including structs in star, then we will have to add optional star expansion here. http://gerrit.cloudera.org:8080/#/c/17847/23/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/17847/23/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@400 PS23, Line 400: itemTupleDesc > I think this function may be called before the expression is analysed, and Yes, I would prefer to try to remove this - it seems generally weird to run this on not yet analyzed expressions. -- To view, visit http://gerrit.cloudera.org:8080/17847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iadb9233677355b85d424cc3f22b00b5a3bf61c57 Gerrit-Change-Number: 17847 Gerrit-PatchSet: 25 Gerrit-Owner: Daniel Becker <[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-Reviewer: Qifan Chen <[email protected]> Gerrit-Comment-Date: Wed, 27 Apr 2022 13:45:06 +0000 Gerrit-HasComments: Yes
