From b5e94b65c00600d10c5440c8f6dbb34755c9fec2 Mon Sep 17 00:00:00 2001 From: seawinde <149132972+seawinde@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:59:54 +0800 Subject: [PATCH] [fix](mtmv) Generate mtmv cache should use ADMIN user, and rewritten plan should not check privilege (#40374) (#41450) ## Proposed changes pr: https://github.com/apache/doris/pull/40374 commitId: f3d92e48 --- .../java/org/apache/doris/catalog/MTMV.java | 8 +- .../privilege/AccessControllerManager.java | 9 + .../rules/analysis/UserAuthentication.java | 6 +- .../exploration/mv/MaterializedViewUtils.java | 9 +- .../mv/MtmvCacheNewConnectContextTest.java | 84 ++++++++ .../with_auth/with_select_table_auth.groovy | 184 ++++++++++++++++++ 6 files changed, 293 insertions(+), 7 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/nereids/mv/MtmvCacheNewConnectContextTest.java create mode 100644 regression-test/suites/nereids_rules_p0/mv/with_auth/with_select_table_auth.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java index 109fcf9628..2f4bef053e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java @@ -307,7 +307,13 @@ public class MTMV extends OlapTable { } // Concurrent situations may result in duplicate cache generation, // but we tolerate this in order to prevent nested use of readLock and write MvLock for the table - MTMVCache mtmvCache = MTMVCache.from(this, connectionContext, true); + MTMVCache mtmvCache; + try { + // Should new context with ADMIN user + mtmvCache = MTMVCache.from(this, MTMVPlanUtil.createMTMVContext(this), true); + } finally { + connectionContext.setThreadLocalInfo(); + } writeMvLock(); try { this.cache = mtmvCache; diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java index 5980c86967..42fa769d03 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java @@ -172,6 +172,15 @@ public class AccessControllerManager { } // ==== Column ==== + // If param has ctx, we can skip auth by isSkipAuth field in ctx + public void checkColumnsPriv(ConnectContext ctx, String ctl, String qualifiedDb, String tbl, Set cols, + PrivPredicate wanted) throws UserException { + if (ctx.isSkipAuth()) { + return; + } + checkColumnsPriv(ctx.getCurrentUserIdentity(), ctl, qualifiedDb, tbl, cols, wanted); + } + public void checkColumnsPriv(UserIdentity currentUser, String ctl, String qualifiedDb, String tbl, Set cols, PrivPredicate wanted) throws UserException { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java index df94a051af..2eca9a45d3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java @@ -17,7 +17,6 @@ package org.apache.doris.nereids.rules.analysis; -import org.apache.doris.analysis.UserIdentity; import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.TableIf; import org.apache.doris.common.ErrorCode; @@ -59,14 +58,13 @@ public class UserAuthentication { } String ctlName = catalog.getName(); AccessControllerManager accessManager = connectContext.getEnv().getAccessManager(); - UserIdentity userIdentity = connectContext.getCurrentUserIdentity(); if (CollectionUtils.isEmpty(columns)) { - if (!accessManager.checkTblPriv(userIdentity, ctlName, dbName, tableName, PrivPredicate.SELECT)) { + if (!accessManager.checkTblPriv(connectContext, ctlName, dbName, tableName, PrivPredicate.SELECT)) { ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLE_ACCESS_DENIED_ERROR, PrivPredicate.SELECT.getPrivs().toString(), tableName); } } else { - accessManager.checkColumnsPriv(userIdentity, ctlName, dbName, tableName, columns, PrivPredicate.SELECT); + accessManager.checkColumnsPriv(connectContext, ctlName, dbName, tableName, columns, PrivPredicate.SELECT); } } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java index 1e6eefbe5b..edd8984ba6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java @@ -227,7 +227,7 @@ public class MaterializedViewUtils { /** * Optimize by rules, this support optimize by custom rules by define different rewriter according to different - * rules + * rules, this method is only for materialized view rewrite */ public static Plan rewriteByRules( CascadesContext cascadesContext, @@ -247,7 +247,12 @@ public class MaterializedViewUtils { CascadesContext rewrittenPlanContext = CascadesContext.initContext( cascadesContext.getStatementContext(), rewrittenPlan, cascadesContext.getCurrentJobContext().getRequiredProperties()); - rewrittenPlan = planRewriter.apply(rewrittenPlanContext); + try { + rewrittenPlanContext.getConnectContext().setSkipAuth(true); + rewrittenPlan = planRewriter.apply(rewrittenPlanContext); + } finally { + rewrittenPlanContext.getConnectContext().setSkipAuth(false); + } Map exprIdToNewRewrittenSlot = Maps.newLinkedHashMap(); for (Slot slot : rewrittenPlan.getOutput()) { exprIdToNewRewrittenSlot.put(slot.getExprId(), slot); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/mv/MtmvCacheNewConnectContextTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/mv/MtmvCacheNewConnectContextTest.java new file mode 100644 index 0000000000..0134d5df4e --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/mv/MtmvCacheNewConnectContextTest.java @@ -0,0 +1,84 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.mv; + +import org.apache.doris.catalog.MTMV; +import org.apache.doris.mtmv.MTMVRelationManager; +import org.apache.doris.nereids.CascadesContext; +import org.apache.doris.nereids.sqltest.SqlTestBase; +import org.apache.doris.nereids.util.PlanChecker; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; + +import mockit.Mock; +import mockit.MockUp; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.BitSet; + +/** + * The connectContext would new instance when generate MTMVCache, after generate, the connectContext should + * reset the connectContext by earlier connectContext to avoid slot id error + * The test is for this. + * */ +public class MtmvCacheNewConnectContextTest extends SqlTestBase { + + @Test + void testConnectContextIsCorrect() throws Exception { + ConnectContext tmp = ConnectContext.get(); + connectContext.getSessionVariable().setDisableNereidsRules("PRUNE_EMPTY_PARTITION"); + BitSet disableNereidsRules = connectContext.getSessionVariable().getDisableNereidsRules(); + new MockUp() { + @Mock + public BitSet getDisableNereidsRules() { + return disableNereidsRules; + } + }; + new MockUp() { + @Mock + public boolean isMVPartitionValid(MTMV mtmv, ConnectContext ctx, boolean forceConsistent) { + return true; + } + }; + connectContext.getSessionVariable().enableMaterializedViewRewrite = true; + connectContext.getSessionVariable().enableMaterializedViewNestRewrite = true; + + createMvByNereids("create materialized view mv1 BUILD IMMEDIATE REFRESH COMPLETE ON MANUAL\n" + + " DISTRIBUTED BY RANDOM BUCKETS 1\n" + + " PROPERTIES ('replication_num' = '1') \n" + + " as select T1.id from T1 inner join T2 " + + " on T1.id = T2.id;"); + CascadesContext c1 = createCascadesContext( + "select T1.id from T1 inner join T2 " + + "on T1.id = T2.id " + + "inner join T3 on T1.id = T3.id", + connectContext + ); + PlanChecker.from(c1) + .analyze() + .rewrite() + .optimize() + .printlnBestPlanTree(); + + ConnectContext now = ConnectContext.get(); + // The connectContext should not change + Assertions.assertSame(tmp, now); + dropMvByNereids("drop materialized view mv1"); + } +} diff --git a/regression-test/suites/nereids_rules_p0/mv/with_auth/with_select_table_auth.groovy b/regression-test/suites/nereids_rules_p0/mv/with_auth/with_select_table_auth.groovy new file mode 100644 index 0000000000..d84d0c6ed2 --- /dev/null +++ b/regression-test/suites/nereids_rules_p0/mv/with_auth/with_select_table_auth.groovy @@ -0,0 +1,184 @@ +package mv.with_auth +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("with_select_table_auth","p0,auth") { + + String db = context.config.getDbNameByFile(context.file) + sql "use ${db}" + sql "set runtime_filter_mode=OFF"; + sql "SET ignore_shape_nodes='PhysicalDistribute,PhysicalProject'" + + String user_name = 'with_select_table_auth' + String pwd = 'test1' + try_sql("DROP USER ${user_name}") + sql """CREATE USER '${user_name}' IDENTIFIED BY '${pwd}'""" + + sql """ + drop table if exists orders + """ + sql """ + CREATE TABLE IF NOT EXISTS orders ( + o_orderkey INTEGER NOT NULL, + o_custkey INTEGER NOT NULL, + o_orderstatus CHAR(1) NOT NULL, + o_totalprice DECIMALV3(15,2) NOT NULL, + o_orderdate DATE NOT NULL, + o_orderpriority CHAR(15) NOT NULL, + o_clerk CHAR(15) NOT NULL, + o_shippriority INTEGER NOT NULL, + O_COMMENT VARCHAR(79) NOT NULL + ) + DUPLICATE KEY(o_orderkey, o_custkey) + PARTITION BY RANGE(o_orderdate) ( + PARTITION `day_2` VALUES LESS THAN ('2023-12-9'), + PARTITION `day_3` VALUES LESS THAN ("2023-12-11"), + PARTITION `day_4` VALUES LESS THAN ("2023-12-30") + ) + DISTRIBUTED BY HASH(o_orderkey) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + + sql """ + drop table if exists lineitem + """ + sql""" + CREATE TABLE IF NOT EXISTS lineitem ( + l_orderkey INTEGER NOT NULL, + l_partkey INTEGER NOT NULL, + l_suppkey INTEGER NOT NULL, + l_linenumber INTEGER NOT NULL, + l_quantity DECIMALV3(15,2) NOT NULL, + l_extendedprice DECIMALV3(15,2) NOT NULL, + l_discount DECIMALV3(15,2) NOT NULL, + l_tax DECIMALV3(15,2) NOT NULL, + l_returnflag CHAR(1) NOT NULL, + l_linestatus CHAR(1) NOT NULL, + l_shipdate DATE NOT NULL, + l_commitdate DATE NOT NULL, + l_receiptdate DATE NOT NULL, + l_shipinstruct CHAR(25) NOT NULL, + l_shipmode CHAR(10) NOT NULL, + l_comment VARCHAR(44) NOT NULL + ) + DUPLICATE KEY(l_orderkey, l_partkey, l_suppkey, l_linenumber) + PARTITION BY RANGE(l_shipdate) ( + PARTITION `day_1` VALUES LESS THAN ('2023-12-9'), + PARTITION `day_2` VALUES LESS THAN ("2023-12-11"), + PARTITION `day_3` VALUES LESS THAN ("2023-12-30")) + DISTRIBUTED BY HASH(l_orderkey) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ) + """ + + sql """ insert into lineitem values + (1, 2, 3, 4, 5.5, 6.5, 7.5, 8.5, 'o', 'k', '2023-12-08', '2023-12-09', '2023-12-10', 'a', 'b', 'yyyyyyyyy'), + (2, 4, 3, 4, 5.5, 6.5, 7.5, 8.5, 'o', 'k', '2023-12-09', '2023-12-09', '2023-12-10', 'a', 'b', 'yyyyyyyyy'), + (3, 2, 4, 4, 5.5, 6.5, 7.5, 8.5, 'o', 'k', '2023-12-10', '2023-12-09', '2023-12-10', 'a', 'b', 'yyyyyyyyy'), + (4, 3, 3, 4, 5.5, 6.5, 7.5, 8.5, 'o', 'k', '2023-12-11', '2023-12-09', '2023-12-10', 'a', 'b', 'yyyyyyyyy'), + (5, 2, 3, 6, 7.5, 8.5, 9.5, 10.5, 'k', 'o', '2023-12-12', '2023-12-12', '2023-12-13', 'c', 'd', 'xxxxxxxxx'); + """ + + sql """ + insert into orders values + (1, 1, 'o', 9.5, '2023-12-08', 'a', 'b', 1, 'yy'), + (1, 1, 'o', 10.5, '2023-12-08', 'a', 'b', 1, 'yy'), + (1, 1, 'o', 10.5, '2023-12-08', 'a', 'b', 1, 'yy'), + (1, 1, 'o', 10.5, '2023-12-08', 'a', 'b', 1, 'yy'), + (2, 1, 'o', 11.5, '2023-12-09', 'a', 'b', 1, 'yy'), + (2, 1, 'o', 11.5, '2023-12-09', 'a', 'b', 1, 'yy'), + (2, 1, 'o', 11.5, '2023-12-09', 'a', 'b', 1, 'yy'), + (3, 1, 'o', 12.5, '2023-12-10', 'a', 'b', 1, 'yy'), + (3, 1, 'o', 12.5, '2023-12-10', 'a', 'b', 1, 'yy'), + (3, 1, 'o', 12.5, '2023-12-10', 'a', 'b', 1, 'yy'), + (3, 1, 'o', 33.5, '2023-12-10', 'a', 'b', 1, 'yy'), + (4, 2, 'o', 43.2, '2023-12-11', 'c','d',2, 'mm'), + (4, 2, 'o', 43.2, '2023-12-11', 'c','d',2, 'mm'), + (4, 2, 'o', 43.2, '2023-12-11', 'c','d',2, 'mm'), + (5, 2, 'o', 56.2, '2023-12-12', 'c','d',2, 'mi'), + (5, 2, 'o', 56.2, '2023-12-12', 'c','d',2, 'mi'), + (5, 2, 'o', 56.2, '2023-12-12', 'c','d',2, 'mi'), + (5, 2, 'o', 1.2, '2023-12-12', 'c','d',2, 'mi'); + """ + + sql """analyze table lineitem with sync""" + sql """analyze table orders with sync""" + + sql """grant select_priv on ${db}.orders to ${user_name}""" + sql """grant select_priv on ${db}.lineitem to ${user_name}""" + sql """grant select_priv on regression_test to ${user_name}""" + + + sql """drop materialized view if exists mv1;""" + sql """ + CREATE MATERIALIZED VIEW ${db}.mv1 + BUILD IMMEDIATE REFRESH AUTO ON MANUAL + DISTRIBUTED BY RANDOM BUCKETS 1 + PROPERTIES ('replication_num' = '1') + AS + select l_shipdate, o_orderdate, l_partkey, l_suppkey, + sum(o_totalprice) as sum_total, + max(o_totalprice) as max_total, + min(o_totalprice) as min_total, + count(*) as count_all, + bitmap_union(to_bitmap(case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end)) as bitmap_union_basic + from lineitem + left join orders on lineitem.l_orderkey = orders.o_orderkey and l_shipdate = o_orderdate + group by + l_shipdate, + o_orderdate, + l_partkey, + l_suppkey; + """ + + sql """analyze table mv1 with sync""" + + connect(user=user_name, password="${pwd}", url=context.config.jdbcUrl) { + sql "use ${db}" + mv_rewrite_success( + """ + select t1.l_partkey, t1.l_suppkey, o_orderdate, + sum(o_totalprice), + max(o_totalprice), + min(o_totalprice), + count(*), + count(distinct case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end) + from (select * from lineitem where l_shipdate = '2023-12-11') t1 + left join orders on t1.l_orderkey = orders.o_orderkey and t1.l_shipdate = o_orderdate + group by + o_orderdate, + l_partkey, + l_suppkey; + """, + "mv1" + ) + } + + connect(user=user_name, password="${pwd}", url=context.config.jdbcUrl) { + sql "use ${db}" + test { + sql """select * from mv1;""" + exception "denied" + } + } + + sql """drop MATERIALIZED VIEW IF EXISTS ${db}.mv1;""" +} +