Steve Carlin 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:

(5 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
Nah, I just meant that if you compare this to the previous code, the view gets 
parsed once at validation time and once again at RelNodeConversion time.  This 
simplified that part of the code a bit.


http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaViewTable.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaViewTable.java:

http://gerrit.cloudera.org:8080/#/c/24211/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaViewTable.java@43
PS5, Line 43:   private final String viewSql_;
> Could call getViewSql() (from the ViewTable class) instead of storing the v
Done


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
Done


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.
Done


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" ?
Done



--
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-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Sun, 07 Jun 2026 04:08:04 +0000
Gerrit-HasComments: Yes

Reply via email to