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 6:

(3 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
> Nah, I just meant that if you compare this to the previous code, the view g
Ack


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@281
PS5, Line 281:       validator.startValidatingView();
> The metadata is stored that way in the database.  So the only real way to f
I don't think we should fix the view string in HMS. That's a bug/behavior of 
Hive and newly created views will still have the same alias issue.

I meant since we get the AST (parsedSqlNode) here, we can travel the tree to 
fix alias issues in place, e.g. for SELECT that has "_cN" alias and has inner 
SELECT clause, use consistent alias "EXPR$N" in outer and inner SELECTs.

It's also 2-pass, but probably need less code?


http://gerrit.cloudera.org:8080/#/c/24211/6/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/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java@288
PS6, Line 288:         parsedSqlNode = validator.validate(parsedSqlNode);
What confused me is how we handle nested views that also have this alias issue. 
Calcite parser throws the exception in the first error, so it seems we can just 
resolve issue of one view. Or do we resolve the issue recursively somewhere 
else?



--
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: 6
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: Mon, 08 Jun 2026 06:36:12 +0000
Gerrit-HasComments: Yes

Reply via email to