Aman Sinha 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 13: (6 comments) 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: vAuthExceptionM > It seems the suffix is not added yet. BTW, I think we can remove the boolea PS 12 was showing the suffix. In any case, in PS 13, I removed the boolean and am using the null check as suggested. http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@931 PS11, Line 931: resolvedTableRef.getTable() instanceof MaterializedViewHdfsTable && > If the MV has masking policies, we won't go to the below check. Should we m Good point. I moved the entire check for source table masking up into an else-if block before going into the check for tableMask.needsMaskingOrFiltering(). http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@322 PS11, Line 322: ((MaterializedViewHdfsTable) tbl).addSrcTables(mvSrcTableNames); > Can we define MV on some real views? In this case, 'mvSrcTableNames' could MVs are not allowed to be defined on regular views since MVs expect the source to be transactional entities and views are not in that category. So, in Hive, the following fails (ran through beeline): 0: jdbc:hive2://localhost:11050/default> create view vv1 as select * from tpch.nation; 0: jdbc:hive2://localhost:11050/default> create materialized view mvv1 as select n_nationkey, count(*) from vv1 group by n_nationkey; Error: Error while compiling statement: FAILED: SemanticException Automatic rewriting for materialized view cannot be enabled if the materialized view uses non-transactional tables (state=42000,code=40000) http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/catalog/MaterializedViewHdfsTable.java File fe/src/main/java/org/apache/impala/catalog/MaterializedViewHdfsTable.java: http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/main/java/org/apache/impala/catalog/MaterializedViewHdfsTable.java@84 PS11, Line 84: } > nit: need a blank line after this Done http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/17595/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3171 PS11, Line 3171: onTable("functional", "mv1_alltypes_jointbl", TPrivilegeLevel.SELECT)); > Can we add an additional test that also add a masking policies on the MV? I Yup, added a column masking policy on mv1_alltypes_jointbl and tested that the same authorization exception is thrown (this after the other change to Analyzer#resolveTableMask()). 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: G > Thanks for the explanation. I will explore the syntax extension for the show stats command in a follow-up jira and we can discuss what would be the appropriate syntax. -- 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: 13 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, 06 Apr 2022 06:40:41 +0000 Gerrit-HasComments: Yes
