Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17595 )
Change subject: IMPALA-10723: Treat materialized view as a table instead of a view ...................................................................... Patch Set 11: (7 comments) Looks great and many thanks for the rework! http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@225 PS11, Line 225: mvAuthException nit. missing suffix _ for these two new fields? http://gerrit.cloudera.org:8080/#/c/17595/7/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/17595/7/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@464 PS7, Line 464: table_ instanceof MaterializedViewHdfsTable > Impala only allows updates/deletes on Kudu tables, so if you do an UPDATE o Done http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java: http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@625 PS11, Line 625: table MV keyword instead? http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@635 PS11, Line 635: drop table functional.materialized_view Should we replace table with keyword MV here? Please refer to my other comment on making a distinction between a table and a MV at SQL level. In this doc, https://docs.cloudera.com/runtime/7.2.6/materialized-view-commands/topics/hive_drop_materialized_view.html, drop MV is used. http://gerrit.cloudera.org:8080/#/c/17595/11/testdata/workloads/functional-planner/queries/PlannerTest/views.test File testdata/workloads/functional-planner/queries/PlannerTest/views.test: http://gerrit.cloudera.org:8080/#/c/17595/11/testdata/workloads/functional-planner/queries/PlannerTest/views.test@589 PS11, Line 589: May add a test to generate a distributed plan for the above query to exercise the distributed planner code. http://gerrit.cloudera.org:8080/#/c/17595/7/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test File testdata/workloads/functional-query/queries/QueryTest/compute-stats.test: http://gerrit.cloudera.org:8080/#/c/17595/7/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test@1841 PS7, Line 1841: table > You mean 'show materialized view stats .. ' ? Since this is now treated a Yeah, I am considering this too, as what it means to treat MVs as tables. It seems to me the idea of this patch is to exploit the benefits of MVs being physically stored as tables. But logically, MVs are still MVs and not tables, at the SQL interface level. Therefore one should be able to create table 'foo' and mv 'foo'. If we follow this idea, then we need to make the distinction for certain SQL statements, such as SHOW TABLE STATS <table> and SHOW MV STATS <mv>, or SHOW CREATE TABLE <table> and SHOW CREATE MV <mv>. http://gerrit.cloudera.org:8080/#/c/17595/11/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test File testdata/workloads/functional-query/queries/QueryTest/compute-stats.test: http://gerrit.cloudera.org:8080/#/c/17595/11/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test@1841 PS11, Line 1841: table See my comment on the distinction between table and MV at SQL level. -- To view, visit http://gerrit.cloudera.org:8080/17595 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3108996124c6544a97fb0c34b6aff5e324a6cff Gerrit-Change-Number: 17595 Gerrit-PatchSet: 11 Gerrit-Owner: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Wed, 23 Mar 2022 13:11:15 +0000 Gerrit-HasComments: Yes
