Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24211 )
Change subject: IMPALA-14909: Calcite planner: Fix validation for views with column list ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/24211/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24211/5//COMMIT_MSG@46 PS5, Line 46: only happens once I think you mean "once per query" here. Catalogd parses the view SQL while loading its metadata: https://github.com/apache/impala/blob/60e2d571389b9aba483a9f98aaa881f0680b9e51/fe/src/main/java/org/apache/impala/catalog/View.java#L105 In the legacy catalog mode, coordinator also parses it once and the View insance can be shared across queries: https://github.com/apache/impala/blob/60e2d571389b9aba483a9f98aaa881f0680b9e51/fe/src/main/java/org/apache/impala/catalog/View.java#L119 However, in local catalog mode, the view SQL is parsed per query, in the LocalCatalog of the query: https://github.com/apache/impala/blob/60e2d571389b9aba483a9f98aaa881f0680b9e51/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java#L46 We might be able to parse it using the calcite parser while loading its metadata. But that's out of the scope of this patch and we can use a separate JIRA for it. http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java: http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java@66 PS5, Line 66: import com.google.common.base.Preconditions; nit: need a blank line after this http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java@278 PS5, Line 278: ParseException nit: don't need this since ImpalaException is a more general exception. http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java@281 PS5, Line 281: parsedSqlNode = validator.validate(parsedSqlNode); Is it ealiser if we fix the "alias issue" before validate()? We can travel parsedSqlNode and transform the "_cN" alias in need. I haven't dig deeper into this. Just curious if we can avoid the two-pass validation. http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ViewAliasCorrector.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ViewAliasCorrector.java: http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ViewAliasCorrector.java@51 PS5, Line 51: the view nit: add "in" ? http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ViewAliasCorrector.java@165 PS5, Line 165: private int aliasCounter_ = 0; nit: unused -- To view, visit http://gerrit.cloudera.org:8080/24211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d0229c8907f69e648034d12fe375d9d9a384e25 Gerrit-Change-Number: 24211 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Fri, 05 Jun 2026 08:48:05 +0000 Gerrit-HasComments: Yes
