From 7ef9e6ec1c8be4c4adf87c0983a2ed91feb24d41 Mon Sep 17 00:00:00 2001 From: Vallish Pai Date: Thu, 14 Nov 2024 10:37:19 +0530 Subject: [PATCH] [Enhancement] (nereids)implement alterRoleCommand in nereids (#43238) Issue Number: close #42799 --- .../org/apache/doris/nereids/DorisParser.g4 | 2 +- .../apache/doris/mysql/privilege/Auth.java | 4 ++ .../nereids/parser/LogicalPlanBuilder.java | 9 +++ .../doris/nereids/trees/plans/PlanType.java | 2 +- .../trees/plans/commands/AlterCommand.java | 45 +++++++++++++ .../plans/commands/AlterRoleCommand.java | 65 ++++++++++++++++++ .../trees/plans/visitor/CommandVisitor.java | 5 ++ .../ddl/alter/test_nereids_role.groovy | 67 +++++++++++++++++++ 8 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterCommand.java create mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoleCommand.java create mode 100644 regression-test/suites/nereids_p0/ddl/alter/test_nereids_role.groovy diff --git a/fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 b/fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 index 3e05afb878a0b6..dd5516cee4b91d 100644 --- a/fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 +++ b/fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 @@ -184,6 +184,7 @@ supportedAlterStatement : ALTER VIEW name=multipartIdentifier (LEFT_PAREN cols=simpleColumnDefs RIGHT_PAREN)? AS query #alterView | ALTER STORAGE VAULT name=multipartIdentifier properties=propertyClause #alterStorageVault + | ALTER ROLE role=identifier commentSpec #alterRole ; supportedDropStatement @@ -558,7 +559,6 @@ unsupportedAlterStatement properties=propertyClause #alterStoragePlicy | ALTER USER (IF EXISTS)? grantUserIdentify passwordOption (COMMENT STRING_LITERAL)? #alterUser - | ALTER ROLE role=identifier commentSpec #alterRole | ALTER REPOSITORY name=identifier properties=propertyClause? #alterRepository ; diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java index cf15dfa2ac9937..3c7ea1376f6aec 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java @@ -1019,6 +1019,10 @@ public void alterRole(AlterRoleStmt stmt) throws DdlException { alterRoleInternal(stmt.getRole(), stmt.getComment(), false); } + public void alterRole(String role, String comment) throws DdlException { + alterRoleInternal(role, comment, false); + } + public void replayCreateRole(PrivInfo info) { try { createRoleInternal(info.getRole(), false, info.getComment(), true); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java index f9603e516872ee..4cd6a381c5e91f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java @@ -53,6 +53,7 @@ import org.apache.doris.nereids.DorisParser.AliasQueryContext; import org.apache.doris.nereids.DorisParser.AliasedQueryContext; import org.apache.doris.nereids.DorisParser.AlterMTMVContext; +import org.apache.doris.nereids.DorisParser.AlterRoleContext; import org.apache.doris.nereids.DorisParser.AlterStorageVaultContext; import org.apache.doris.nereids.DorisParser.AlterViewContext; import org.apache.doris.nereids.DorisParser.ArithmeticBinaryContext; @@ -394,6 +395,7 @@ import org.apache.doris.nereids.trees.plans.algebra.SetOperation.Qualifier; import org.apache.doris.nereids.trees.plans.commands.AddConstraintCommand; import org.apache.doris.nereids.trees.plans.commands.AlterMTMVCommand; +import org.apache.doris.nereids.trees.plans.commands.AlterRoleCommand; import org.apache.doris.nereids.trees.plans.commands.AlterStorageVaultCommand; import org.apache.doris.nereids.trees.plans.commands.AlterViewCommand; import org.apache.doris.nereids.trees.plans.commands.CallCommand; @@ -4077,8 +4079,15 @@ public LogicalPlan visitShowProc(ShowProcContext ctx) { return new ShowProcCommand(path); } + @Override public LogicalPlan visitShowCreateMaterializedView(ShowCreateMaterializedViewContext ctx) { List nameParts = visitMultipartIdentifier(ctx.tableName); return new ShowCreateMaterializedViewCommand(stripQuotes(ctx.mvName.getText()), new TableNameInfo(nameParts)); } + + @Override + public LogicalPlan visitAlterRole(AlterRoleContext ctx) { + String comment = visitCommentSpec(ctx.commentSpec()); + return new AlterRoleCommand(ctx.role.getText(), comment); + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java index 9c4e66d206cedb..5650e1466d60f3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java @@ -161,6 +161,7 @@ public enum PlanType { SHOW_PROCEDURE_COMMAND, SHOW_CREATE_PROCEDURE_COMMAND, CREATE_VIEW_COMMAND, + ALTER_ROLE_COMMAND, ALTER_VIEW_COMMAND, ALTER_STORAGE_VAULT, DROP_CATALOG_RECYCLE_BIN_COMMAND, @@ -172,7 +173,6 @@ public enum PlanType { SET_TRANSACTION_COMMAND, SET_USER_PROPERTIES_COMMAND, SET_DEFAULT_STORAGE_VAULT_COMMAND, - PREPARED_COMMAND, EXECUTE_COMMAND, SHOW_CONFIG_COMMAND, diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterCommand.java new file mode 100644 index 00000000000000..fc94bd167ac648 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterCommand.java @@ -0,0 +1,45 @@ +// 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.trees.plans.commands; + +import org.apache.doris.analysis.StmtType; +import org.apache.doris.nereids.trees.plans.PlanType; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.StmtExecutor; + +/** + * base class for all Alter commands + */ +public abstract class AlterCommand extends Command implements ForwardWithSync { + public AlterCommand(PlanType type) { + super(type); + } + + @Override + public StmtType stmtType() { + return StmtType.ALTER; + } + + @Override + public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { + doRun(ctx, executor); + } + + public abstract void doRun(ConnectContext ctx, StmtExecutor executor) throws Exception; + +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoleCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoleCommand.java new file mode 100644 index 00000000000000..fe8d0fd5db95a1 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoleCommand.java @@ -0,0 +1,65 @@ +// 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.trees.plans.commands; + +import org.apache.doris.catalog.Env; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.nereids.trees.plans.PlanType; +import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.StmtExecutor; + +import com.google.common.base.Strings; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * alter role command + */ +public class AlterRoleCommand extends AlterCommand { + public static final Logger LOG = LogManager.getLogger(AlterRoleCommand.class); + private final String role; + private final String comment; + + /** + * constructor + */ + + public AlterRoleCommand(String role, String comment) { + super(PlanType.ALTER_ROLE_COMMAND); + this.role = role; + this.comment = Strings.nullToEmpty(comment); + + } + + @Override + public void doRun(ConnectContext ctx, StmtExecutor executor) throws Exception { + // check if current user has GRANT priv on GLOBAL level. + if (!Env.getCurrentEnv().getAccessManager().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) { + ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "ALTER ROLE"); + } + Env.getCurrentEnv().getAuth().alterRole(role, comment); + } + + @Override + public R accept(PlanVisitor visitor, C context) { + return visitor.visitAlterRoleCommand(this, context); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/CommandVisitor.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/CommandVisitor.java index a74f9bde93a1fc..4cf560dd6187a3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/CommandVisitor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/CommandVisitor.java @@ -19,6 +19,7 @@ import org.apache.doris.nereids.trees.plans.commands.AddConstraintCommand; import org.apache.doris.nereids.trees.plans.commands.AlterMTMVCommand; +import org.apache.doris.nereids.trees.plans.commands.AlterRoleCommand; import org.apache.doris.nereids.trees.plans.commands.AlterViewCommand; import org.apache.doris.nereids.trees.plans.commands.CallCommand; import org.apache.doris.nereids.trees.plans.commands.CancelMTMVTaskCommand; @@ -263,4 +264,8 @@ default R visitShowCreateMaterializedViewCommand(ShowCreateMaterializedViewComma C context) { return visitCommand(showCreateMtlzViewCommand, context); } + + default R visitAlterRoleCommand(AlterRoleCommand alterRoleCommand, C context) { + return visitCommand(alterRoleCommand, context); + } } diff --git a/regression-test/suites/nereids_p0/ddl/alter/test_nereids_role.groovy b/regression-test/suites/nereids_p0/ddl/alter/test_nereids_role.groovy new file mode 100644 index 00000000000000..cb3d2f62ecd3c7 --- /dev/null +++ b/regression-test/suites/nereids_p0/ddl/alter/test_nereids_role.groovy @@ -0,0 +1,67 @@ +// 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. + +import org.junit.Assert; + +suite("test_nereids_role", "account") { + def role= 'account_role_test' + def user = 'acount_role_user_test' + def dbName = 'account_role_test_db' + def pwd = 'C123_567p' + + try_sql("DROP ROLE ${role}") + try_sql("DROP USER ${user}") + sql """DROP DATABASE IF EXISTS ${dbName}""" + sql """CREATE DATABASE ${dbName}""" + + sql """CREATE ROLE ${role}""" + sql """GRANT SELECT_PRIV ON ${context.config.defaultDb} TO ROLE '${role}'""" + sql """GRANT SELECT_PRIV ON ${dbName} TO ROLE '${role}'""" + sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}' DEFAULT ROLE '${role}'""" + def result1 = connect(user=user, password="${pwd}", url=context.config.jdbcUrl) { + sql "show databases like '${dbName}'" + } + assertEquals(result1.size(), 1) + + sql """REVOKE SELECT_PRIV ON ${dbName} FROM ROLE '${role}'""" + def result2 = connect(user=user, password="${pwd}", url=context.config.jdbcUrl) { + sql "show databases like '${dbName}'" + } + assertEquals(result2.size(), 0) + + sql """DROP USER ${user}""" + sql """DROP ROLE ${role}""" + sql """DROP DATABASE ${dbName}""" + + // test comment + // create role with comment + sql """CREATE ROLE ${role} comment 'account_p0_account_role_test_comment_create'""" + def roles_create = sql """show roles""" + logger.info("roles_create: " + roles_create.toString()) + assertTrue(roles_create.toString().contains("account_p0_account_role_test_comment_create")) + // alter role with comment + checkNereidsExecute("ALTER ROLE ${role} comment 'account_p0_account_role_test_comment_alter';"); + def roles_alter = sql """show roles""" + logger.info("roles_alter: " + roles_alter.toString()) + assertTrue(roles_alter.toString().contains("account_p0_account_role_test_comment_alter")) + // drop role + sql """DROP ROLE ${role}""" + def roles_drop = sql """show roles""" + logger.info("roles_drop: " + roles_drop.toString()) + assertFalse(roles_drop.toString().contains("account_p0_account_role_test_comment_alter")) +} +