Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/22883 )
Change subject: IMPALA-13982: Support regular views for Calcite planner in Impala ...................................................................... Patch Set 12: (2 comments) In patch set 12, I addressed Steve's comments on patch set 11. On the other hand, I will also try to figure out why sqlValidator_.validate() in CalciteAnalysisDriver#analyze() could succeed even if we don't add the referenced HDFS tables by a regular view. This is related to authorization of a regular view in that we would like to collect the underlying tables of a regular view if this view was not created by a superuser. http://gerrit.cloudera.org:8080/#/c/22883/11/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/22883/11/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisDriver.java@104 PS11, Line 104: stmtTableCache_); > Is there a reason you changed this? I don't have strong feelings about it Thanks Steve! It turns out that when CalciteRelNodeConverter#convert() is called to convert the valid AST into a logical plan, under the hood ViewTable#expandView() in https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/schema/impl/ViewTable.java will be called. expandView() has to validate the SQL statement defining the view. If we do not add all the tables in 'stmtTableCache_', then we will encounter a validation error like the following when submitting a query involving regular views. [localhost:21050] default> select id from functional.alltypes_view; Query: select id from functional.alltypes_view Query submitted at: 2025-05-21 16:25:49 (Coordinator: http://fangyu:25000) ERROR: Query 7f4dd8307fedac8c:10eb8aed00000000 failed: RuntimeException: Error while parsing view definition: SELECT * FROM functional.alltypes CAUSED BY: CalciteContextException: From line 1, column 15 to line 1, column 33: Object 'ALLTYPES' not found within 'functional' CAUSED BY: SqlValidatorException: Object 'ALLTYPES' not found within 'functional' http://gerrit.cloudera.org:8080/#/c/22883/11/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java: http://gerrit.cloudera.org:8080/#/c/22883/11/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@68 PS11, Line 68: private final RelOptTable.ViewExpander viewExpander_; > Nit: we should probably use lower camel case since this is now non-static. Thanks Steve! I will address this in the next patch. -- To view, visit http://gerrit.cloudera.org:8080/22883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I600aae816727ae942fb221fae84c2aac63ae1893 Gerrit-Change-Number: 22883 Gerrit-PatchSet: 12 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Thu, 22 May 2025 00:01:14 +0000 Gerrit-HasComments: Yes
