This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.1 by this push:
     new 45e1f9f4c4c [branch-2.1](cherry-pick)Role comment 2.1 #32837
45e1f9f4c4c is described below

commit 45e1f9f4c4c596708a5a467395d2f47c5f3e3366
Author: zhangdong <493738...@qq.com>
AuthorDate: Wed Mar 27 08:33:54 2024 +0800

    [branch-2.1](cherry-pick)Role comment 2.1 #32837
---
 fe/fe-core/src/main/cup/sql_parser.cup             |  27 +++++-
 .../{CreateRoleStmt.java => AlterRoleStmt.java}    |  32 +++----
 .../org/apache/doris/analysis/AlterUserStmt.java   |  17 +++-
 .../org/apache/doris/analysis/CreateRoleStmt.java  |  20 +++-
 .../org/apache/doris/analysis/CreateUserStmt.java  |  16 +++-
 .../org/apache/doris/analysis/ShowRolesStmt.java   |   1 +
 .../org/apache/doris/common/proc/AuthProcDir.java  |   5 +-
 .../org/apache/doris/journal/JournalEntity.java    |   1 +
 .../org/apache/doris/mysql/privilege/Auth.java     | 105 +++++++++++++++------
 .../org/apache/doris/mysql/privilege/Role.java     |  16 ++++
 .../apache/doris/mysql/privilege/RoleManager.java  |   1 +
 .../org/apache/doris/mysql/privilege/User.java     |  13 ++-
 .../apache/doris/mysql/privilege/UserManager.java  |  10 +-
 .../doris/persist/AlterUserOperationLog.java       |  10 +-
 .../java/org/apache/doris/persist/EditLog.java     |   9 ++
 .../org/apache/doris/persist/OperationType.java    |   2 +
 .../java/org/apache/doris/persist/PrivInfo.java    |  17 ++++
 .../main/java/org/apache/doris/qe/DdlExecutor.java |   3 +
 .../org/apache/doris/mysql/privilege/AuthTest.java |   4 +-
 .../suites/account_p0/test_account.groovy          |  23 ++++-
 .../suites/account_p0/test_revoke_role.groovy      |   4 +-
 regression-test/suites/account_p0/test_role.groovy |  19 ++++
 22 files changed, 280 insertions(+), 75 deletions(-)

diff --git a/fe/fe-core/src/main/cup/sql_parser.cup 
b/fe/fe-core/src/main/cup/sql_parser.cup
index f907b762407..8d6c43ff6c2 100644
--- a/fe/fe-core/src/main/cup/sql_parser.cup
+++ b/fe/fe-core/src/main/cup/sql_parser.cup
@@ -953,7 +953,7 @@ nonterminal JobName job_name;
 nonterminal Map<String, String> with_analysis_properties;
 nonterminal List<Map<String, String>> opt_with_analysis_properties;
 
-nonterminal String opt_db, procedure_or_function, opt_comment, opt_engine;
+nonterminal String opt_db, procedure_or_function, opt_comment, 
opt_comment_null, opt_engine;
 nonterminal ColumnDef.DefaultValue opt_default_value;
 nonterminal Boolean opt_if_exists, opt_if_not_exists;
 nonterminal Boolean opt_external;
@@ -1427,8 +1427,9 @@ alter_stmt ::=
       opt_if_exists:ifExists
       grant_user:user
       opt_password_option:passwdOptions
+      opt_comment_null:comment
     {:
-        RESULT = new AlterUserStmt(ifExists, user, null, passwdOptions);
+        RESULT = new AlterUserStmt(ifExists, user, null, passwdOptions, 
comment);
     :}
     | KW_ALTER KW_REPOSITORY ident:repoName opt_properties:properties
     {:
@@ -1877,8 +1878,9 @@ create_stmt ::=
       grant_user:user
       opt_user_role:userRole
       opt_password_option:passwdOptions
+      opt_comment:comment
     {:
-        RESULT = new CreateUserStmt(ifNotExists, user, userRole, 
passwdOptions);
+        RESULT = new CreateUserStmt(ifNotExists, user, userRole, 
passwdOptions, comment);
     :}
     | KW_CREATE KW_VIEW opt_if_not_exists:ifNotExists table_name:viewName
         opt_col_with_comment_list:columns opt_comment:comment KW_AS 
query_stmt:view_def
@@ -1889,9 +1891,13 @@ create_stmt ::=
     {:
         RESULT = new CreateRepositoryStmt(isReadOnly, repoName, storage);
     :}
-    | KW_CREATE KW_ROLE opt_if_not_exists:ifNotExists ident:role
+    | KW_CREATE KW_ROLE opt_if_not_exists:ifNotExists ident:role 
opt_comment:comment
     {:
-        RESULT = new CreateRoleStmt(ifNotExists, role);
+        RESULT = new CreateRoleStmt(ifNotExists, role, comment);
+    :}
+    | KW_ALTER KW_ROLE ident:role KW_COMMENT STRING_LITERAL:comment
+    {:
+        RESULT = new AlterRoleStmt(role, comment);
     :}
     | KW_CREATE KW_FILE STRING_LITERAL:fileName opt_db:db KW_PROPERTIES LPAREN 
key_value_map:properties RPAREN
     {:
@@ -3800,6 +3806,17 @@ opt_comment ::=
     :}
     ;
 
+opt_comment_null ::=
+    /* empty */
+    {:
+        RESULT = null;
+    :}
+    | KW_COMMENT STRING_LITERAL:comment
+    {:
+        RESULT = comment;
+    :}
+    ;
+
 opt_index_type ::=
     {:
         RESULT = null;
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterRoleStmt.java
similarity index 75%
copy from fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
copy to fe/fe-core/src/main/java/org/apache/doris/analysis/AlterRoleStmt.java
index bd19325f1ef..75e86a4b6f0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterRoleStmt.java
@@ -20,46 +20,46 @@ package org.apache.doris.analysis;
 import org.apache.doris.catalog.Env;
 import org.apache.doris.common.ErrorCode;
 import org.apache.doris.common.ErrorReport;
-import org.apache.doris.common.FeNameFormat;
 import org.apache.doris.common.UserException;
 import org.apache.doris.mysql.privilege.PrivPredicate;
 import org.apache.doris.qe.ConnectContext;
 
-public class CreateRoleStmt extends DdlStmt {
+import com.google.common.base.Strings;
+
+public class AlterRoleStmt extends DdlStmt {
 
-    private boolean ifNotExists;
     private String role;
 
-    public CreateRoleStmt(String role) {
-        this.role = role;
-    }
+    private String comment;
 
-    public CreateRoleStmt(boolean ifNotExists, String role) {
-        this.ifNotExists = ifNotExists;
+    public AlterRoleStmt(String role, String comment) {
         this.role = role;
-    }
-
-    public boolean isSetIfNotExists() {
-        return ifNotExists;
+        this.comment = Strings.nullToEmpty(comment);
     }
 
     public String getRole() {
         return role;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     @Override
     public void analyze(Analyzer analyzer) throws UserException {
         super.analyze(analyzer);
-        FeNameFormat.checkRoleName(role, false /* can not be admin */, "Can 
not create role");
-
         // 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, 
"CREATE ROLE");
+            
ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, 
"ALTER ROLE");
         }
     }
 
     @Override
     public String toSql() {
-        return "CREATE ROLE " + role;
+        StringBuilder sb = new StringBuilder();
+        sb.append("ALTER ROLE ");
+        sb.append(role);
+        sb.append(" COMMENT \"").append(comment).append("\"");
+        return sb.toString();
     }
 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterUserStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterUserStmt.java
index 6495f73fc94..544a106b3ba 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterUserStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterUserStmt.java
@@ -44,22 +44,27 @@ public class AlterUserStmt extends DdlStmt {
     private String role;
     private PasswordOptions passwordOptions;
 
+    private String comment;
+
     // Only support doing one of these operation at one time.
     public enum OpType {
         SET_PASSWORD,
         SET_ROLE,
         SET_PASSWORD_POLICY,
         LOCK_ACCOUNT,
-        UNLOCK_ACCOUNT
+        UNLOCK_ACCOUNT,
+        MODIFY_COMMENT
     }
 
     private Set<OpType> ops = Sets.newHashSet();
 
-    public AlterUserStmt(boolean ifExist, UserDesc userDesc, String role, 
PasswordOptions passwordOptions) {
+    public AlterUserStmt(boolean ifExist, UserDesc userDesc, String role, 
PasswordOptions passwordOptions,
+            String comment) {
         this.ifExist = ifExist;
         this.userDesc = userDesc;
         this.role = role;
         this.passwordOptions = passwordOptions;
+        this.comment = comment;
     }
 
     public boolean isIfExist() {
@@ -90,6 +95,10 @@ public class AlterUserStmt extends DdlStmt {
         return ops.iterator().next();
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     @Override
     public void analyze(Analyzer analyzer) throws UserException {
         super.analyze(analyzer);
@@ -104,6 +113,10 @@ public class AlterUserStmt extends DdlStmt {
             ops.add(OpType.SET_ROLE);
         }
 
+        // may be set comment to "", so not use `Strings.isNullOrEmpty`
+        if (comment != null) {
+            ops.add(OpType.MODIFY_COMMENT);
+        }
         passwordOptions.analyze();
         if (passwordOptions.getAccountUnlocked() == 
FailedLoginPolicy.LOCK_ACCOUNT) {
             throw new AnalysisException("Not support lock account now");
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
index bd19325f1ef..0a60a3060c3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
@@ -25,18 +25,24 @@ import org.apache.doris.common.UserException;
 import org.apache.doris.mysql.privilege.PrivPredicate;
 import org.apache.doris.qe.ConnectContext;
 
+import com.google.common.base.Strings;
+import org.apache.commons.lang3.StringUtils;
+
 public class CreateRoleStmt extends DdlStmt {
 
     private boolean ifNotExists;
     private String role;
 
+    private String comment;
+
     public CreateRoleStmt(String role) {
         this.role = role;
     }
 
-    public CreateRoleStmt(boolean ifNotExists, String role) {
+    public CreateRoleStmt(boolean ifNotExists, String role, String comment) {
         this.ifNotExists = ifNotExists;
         this.role = role;
+        this.comment = Strings.nullToEmpty(comment);
     }
 
     public boolean isSetIfNotExists() {
@@ -47,6 +53,10 @@ public class CreateRoleStmt extends DdlStmt {
         return role;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     @Override
     public void analyze(Analyzer analyzer) throws UserException {
         super.analyze(analyzer);
@@ -60,6 +70,12 @@ public class CreateRoleStmt extends DdlStmt {
 
     @Override
     public String toSql() {
-        return "CREATE ROLE " + role;
+        StringBuilder sb = new StringBuilder();
+        sb.append("CREATE ROLE ");
+        sb.append(role);
+        if (!StringUtils.isEmpty(comment)) {
+            sb.append(" COMMENT \"").append(comment).append("\"");
+        }
+        return sb.toString();
     }
 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateUserStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateUserStmt.java
index e3d5dc5556d..d8c589bf0b7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateUserStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateUserStmt.java
@@ -27,6 +27,7 @@ import org.apache.doris.mysql.privilege.Role;
 import org.apache.doris.qe.ConnectContext;
 
 import com.google.common.base.Strings;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -52,6 +53,7 @@ public class CreateUserStmt extends DdlStmt {
     private PassVar passVar;
     private String role;
     private PasswordOptions passwordOptions;
+    private String comment;
 
     public CreateUserStmt() {
     }
@@ -65,10 +67,11 @@ public class CreateUserStmt extends DdlStmt {
     }
 
     public CreateUserStmt(boolean ifNotExist, UserDesc userDesc, String role) {
-        this(ifNotExist, userDesc, role, null);
+        this(ifNotExist, userDesc, role, null, null);
     }
 
-    public CreateUserStmt(boolean ifNotExist, UserDesc userDesc, String role, 
PasswordOptions passwordOptions) {
+    public CreateUserStmt(boolean ifNotExist, UserDesc userDesc, String role, 
PasswordOptions passwordOptions,
+            String comment) {
         this.ifNotExist = ifNotExist;
         userIdent = userDesc.getUserIdent();
         passVar = userDesc.getPassVar();
@@ -77,6 +80,7 @@ public class CreateUserStmt extends DdlStmt {
         if (this.passwordOptions == null) {
             this.passwordOptions = PasswordOptions.UNSET_OPTION;
         }
+        this.comment = Strings.nullToEmpty(comment);
     }
 
     public boolean isIfNotExist() {
@@ -104,6 +108,10 @@ public class CreateUserStmt extends DdlStmt {
         return passwordOptions;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     @Override
     public void analyze(Analyzer analyzer) throws UserException {
         super.analyze(analyzer);
@@ -149,7 +157,9 @@ public class CreateUserStmt extends DdlStmt {
         if (passwordOptions != null) {
             sb.append(passwordOptions.toSql());
         }
-
+        if (!StringUtils.isEmpty(comment)) {
+            sb.append(" COMMENT \"").append(comment).append("\"");
+        }
         return sb.toString();
     }
 
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowRolesStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowRolesStmt.java
index fdeaeaea23d..52c5bd75765 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowRolesStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowRolesStmt.java
@@ -34,6 +34,7 @@ public class ShowRolesStmt extends ShowStmt {
         ShowResultSetMetaData.Builder builder = 
ShowResultSetMetaData.builder();
 
         builder.addColumn(new Column("Name", ScalarType.createVarchar(100)));
+        builder.addColumn(new Column("Comment", 
ScalarType.createVarchar(100)));
         builder.addColumn(new Column("Users", ScalarType.createVarchar(100)));
         builder.addColumn(new Column("GlobalPrivs", 
ScalarType.createVarchar(300)));
         builder.addColumn(new Column("CatalogPrivs", 
ScalarType.createVarchar(300)));
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java 
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java
index 6adc86ee269..95434fec723 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java
@@ -31,8 +31,9 @@ import com.google.common.collect.ImmutableList;
  */
 public class AuthProcDir implements ProcDirInterface {
     public static final ImmutableList<String> TITLE_NAMES = new 
ImmutableList.Builder<String>()
-            
.add("UserIdentity").add("Password").add("Roles").add("GlobalPrivs").add("CatalogPrivs")
-            
.add("DatabasePrivs").add("TablePrivs").add("ColPrivs").add("ResourcePrivs").add("WorkloadGroupPrivs")
+            
.add("UserIdentity").add("Comment").add("Password").add("Roles").add("GlobalPrivs").add("CatalogPrivs")
+            
.add("DatabasePrivs").add("TablePrivs").add("ColPrivs").add("ResourcePrivs").add("CloudClusterPrivs")
+            .add("WorkloadGroupPrivs")
             .build();
 
     private Auth auth;
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java 
b/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
index 8f0d2b54580..1c3f7444dd9 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
@@ -386,6 +386,7 @@ public class JournalEntity implements Writable {
             case OperationType.OP_REVOKE_PRIV:
             case OperationType.OP_SET_PASSWORD:
             case OperationType.OP_CREATE_ROLE:
+            case OperationType.OP_ALTER_ROLE:
             case OperationType.OP_DROP_ROLE: {
                 data = PrivInfo.read(in);
                 isRead = true;
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 eab88b43189..182863e7ac9 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
@@ -17,6 +17,7 @@
 
 package org.apache.doris.mysql.privilege;
 
+import org.apache.doris.analysis.AlterRoleStmt;
 import org.apache.doris.analysis.AlterUserStmt;
 import org.apache.doris.analysis.AlterUserStmt.OpType;
 import org.apache.doris.analysis.CreateRoleStmt;
@@ -94,7 +95,7 @@ public class Auth implements Writable {
     public static final String UNKNOWN_USER = "unknown";
     public static final String DEFAULT_CATALOG = 
InternalCatalog.INTERNAL_CATALOG_NAME;
 
-    //There is no concurrency control logic inside 
roleManager,userManager,userRoleManage and rpropertyMgr,
+    // There is no concurrency control logic inside 
roleManager,userManager,userRoleManage and rpropertyMgr,
     // and it is completely managed by Auth.
     // Therefore, their methods cannot be directly called outside, and should 
be called indirectly through Auth.
     private RoleManager roleManager = new RoleManager();
@@ -423,20 +424,21 @@ public class Auth implements Writable {
     // create user
     public void createUser(CreateUserStmt stmt) throws DdlException {
         createUserInternal(stmt.getUserIdent(), stmt.getQualifiedRole(),
-                stmt.getPassword(), stmt.isIfNotExist(), 
stmt.getPasswordOptions(), false);
+                stmt.getPassword(), stmt.isIfNotExist(), 
stmt.getPasswordOptions(), stmt.getComment(), false);
     }
 
     public void replayCreateUser(PrivInfo privInfo) {
         try {
             createUserInternal(privInfo.getUserIdent(), privInfo.getRole(), 
privInfo.getPasswd(), false,
-                    privInfo.getPasswordOptions(), true);
+                    privInfo.getPasswordOptions(), privInfo.getComment(), 
true);
         } catch (DdlException e) {
             LOG.error("should not happen", e);
         }
     }
 
     private void createUserInternal(UserIdentity userIdent, String roleName, 
byte[] password,
-            boolean ignoreIfExists, PasswordOptions passwordOptions, boolean 
isReplay) throws DdlException {
+            boolean ignoreIfExists, PasswordOptions passwordOptions, String 
comment, boolean isReplay)
+            throws DdlException {
         writeLock();
         try {
             // check if role exist
@@ -460,7 +462,7 @@ public class Auth implements Writable {
             // create user
             try {
                 // we should not throw AnalysisException at here,so transfer it
-                userManager.createUser(userIdent, password, null, false);
+                userManager.createUser(userIdent, password, null, false, 
comment);
             } catch (PatternMatcherException e) {
                 throw new DdlException("create user failed,", e);
             }
@@ -468,7 +470,7 @@ public class Auth implements Writable {
                 // save password to password history
                 passwdPolicyManager.updatePassword(userIdent, password);
             }
-            //4.create defaultRole
+            // 4.create defaultRole
             Role defaultRole = roleManager.createDefaultRole(userIdent);
             // 5.create user role
             userRoleManager.addUserRole(userIdent, defaultRole.getRoleName());
@@ -482,7 +484,7 @@ public class Auth implements Writable {
             passwdPolicyManager.updatePolicy(userIdent, password, 
passwordOptions);
 
             if (!isReplay) {
-                PrivInfo privInfo = new PrivInfo(userIdent, null, password, 
roleName, passwordOptions);
+                PrivInfo privInfo = new PrivInfo(userIdent, null, password, 
roleName, passwordOptions, comment);
                 Env.getCurrentEnv().getEditLog().logCreateUser(privInfo);
             }
             LOG.info("finished to create user: {}, is replay: {}", userIdent, 
isReplay);
@@ -517,7 +519,7 @@ public class Auth implements Writable {
 
             // drop default role
             roleManager.removeDefaultRole(userIdent);
-            //drop user role
+            // drop user role
             userRoleManager.dropUser(userIdent);
             passwdPolicyManager.dropUser(userIdent);
             userManager.removeUser(userIdent);
@@ -576,7 +578,7 @@ public class Auth implements Writable {
     }
 
     // grant for TablePattern
-    //if no role,role is default role of userIdent
+    // if no role,role is default role of userIdent
     private void grantInternal(UserIdentity userIdent, String role, 
TablePattern tblPattern, PrivBitSet privs,
             Map<ColPrivilegeKey, Set<String>> colPrivileges, boolean 
errOnNonExist, boolean isReplay)
             throws DdlException {
@@ -650,7 +652,7 @@ public class Auth implements Writable {
             if (userManager.getUserByUserIdentity(userIdent) == null) {
                 throw new DdlException("user: " + userIdent + " does not 
exist");
             }
-            //roles must exist
+            // roles must exist
             for (String roleName : roles) {
                 if (roleManager.getRole(roleName) == null) {
                     throw new DdlException("role:" + roleName + " does not 
exist");
@@ -794,7 +796,7 @@ public class Auth implements Writable {
             if (userManager.getUserByUserIdentity(userIdent) == null) {
                 throw new DdlException("user: " + userIdent + " does not 
exist");
             }
-            //roles must exist
+            // roles must exist
             for (String roleName : roles) {
                 if (roleManager.getRole(roleName) == null) {
                     throw new DdlException("role:" + roleName + " does not 
exist");
@@ -873,19 +875,44 @@ public class Auth implements Writable {
 
     // create role
     public void createRole(CreateRoleStmt stmt) throws DdlException {
-        createRoleInternal(stmt.getRole(), stmt.isSetIfNotExists(), false);
+        createRoleInternal(stmt.getRole(), stmt.isSetIfNotExists(), 
stmt.getComment(), false);
+    }
+
+    public void alterRole(AlterRoleStmt stmt) throws DdlException {
+        alterRoleInternal(stmt.getRole(), stmt.getComment(), false);
     }
 
     public void replayCreateRole(PrivInfo info) {
         try {
-            createRoleInternal(info.getRole(), false, true);
+            createRoleInternal(info.getRole(), false, info.getComment(), true);
         } catch (DdlException e) {
             LOG.error("should not happened", e);
         }
     }
 
-    private void createRoleInternal(String role, boolean ignoreIfExists, 
boolean isReplay) throws DdlException {
-        Role emptyPrivsRole = new Role(role);
+    public void replayAlterRole(PrivInfo info) {
+        try {
+            alterRoleInternal(info.getRole(), info.getComment(), true);
+        } catch (DdlException e) {
+            LOG.error("should not happened", e);
+        }
+    }
+
+    private void alterRoleInternal(String roleName, String comment, boolean 
isReplay) throws DdlException {
+        Role role = roleManager.getRole(roleName);
+        if (role == null) {
+            throw new DdlException("Role " + roleName + " not exist");
+        }
+        role.setComment(comment);
+        if (!isReplay) {
+            PrivInfo info = new PrivInfo(roleName, comment);
+            Env.getCurrentEnv().getEditLog().logAlterRole(info);
+        }
+    }
+
+    private void createRoleInternal(String role, boolean ignoreIfExists, 
String comment, boolean isReplay)
+            throws DdlException {
+        Role emptyPrivsRole = new Role(role, comment);
         writeLock();
         try {
             if (ignoreIfExists && roleManager.getRole(role) != null) {
@@ -896,7 +923,7 @@ public class Auth implements Writable {
             roleManager.addOrMergeRole(emptyPrivsRole, true /* err on exist 
*/);
 
             if (!isReplay) {
-                PrivInfo info = new PrivInfo(null, null, null, role, null);
+                PrivInfo info = new PrivInfo(role, comment);
                 Env.getCurrentEnv().getEditLog().logCreateRole(info);
             }
         } finally {
@@ -1132,6 +1159,8 @@ public class Auth implements Writable {
         // ================= UserIdentity =======================
         userAuthInfo.add(userIdent.toString());
         if (isLdapAuthEnabled() && 
ldapManager.doesUserExist(userIdent.getQualifiedUser())) {
+            // ============== Comment ==============
+            userAuthInfo.add(FeConstants.null_string);
             LdapUserInfo ldapUserInfo = 
ldapManager.getUserInfo(userIdent.getQualifiedUser());
             // ============== Password ==============
             userAuthInfo.add(ldapUserInfo.isSetPasswd() ? "Yes" : "No");
@@ -1143,7 +1172,10 @@ public class Auth implements Writable {
             if (user == null) {
                 userAuthInfo.add(FeConstants.null_string);
                 userAuthInfo.add(FeConstants.null_string);
+                userAuthInfo.add(FeConstants.null_string);
             } else {
+                // ============== Comment ==============
+                userAuthInfo.add(user.getComment());
                 // ============== Password ==============
                 userAuthInfo.add(user.hasPassword() ? "Yes" : "No");
                 // ============== Roles ==============
@@ -1321,11 +1353,11 @@ public class Auth implements Writable {
             UserIdentity rootUser = new UserIdentity(ROOT_USER, "%");
             rootUser.setIsAnalyzed();
             createUserInternal(rootUser, Role.OPERATOR_ROLE, new byte[0],
-                    false /* ignore if exists */, 
PasswordOptions.UNSET_OPTION, true /* is replay */);
+                    false /* ignore if exists */, 
PasswordOptions.UNSET_OPTION, "ROOT", true /* is replay */);
             UserIdentity adminUser = new UserIdentity(ADMIN_USER, "%");
             adminUser.setIsAnalyzed();
             createUserInternal(adminUser, Role.ADMIN_ROLE, new byte[0],
-                    false /* ignore if exists */, 
PasswordOptions.UNSET_OPTION, true /* is replay */);
+                    false /* ignore if exists */, 
PasswordOptions.UNSET_OPTION, "ADMIN", true /* is replay */);
         } catch (DdlException e) {
             LOG.error("should not happened", e);
         }
@@ -1524,20 +1556,20 @@ public class Auth implements Writable {
 
     public void alterUser(AlterUserStmt stmt) throws DdlException {
         alterUserInternal(stmt.isIfExist(), stmt.getOpType(), 
stmt.getUserIdent(), stmt.getPassword(), stmt.getRole(),
-                stmt.getPasswordOptions(), false);
+                stmt.getPasswordOptions(), stmt.getComment(), false);
     }
 
     public void replayAlterUser(AlterUserOperationLog log) {
         try {
             alterUserInternal(true, log.getOp(), log.getUserIdent(), 
log.getPassword(), log.getRole(),
-                    log.getPasswordOptions(), true);
+                    log.getPasswordOptions(), log.getComment(), true);
         } catch (DdlException e) {
             LOG.error("should not happen", e);
         }
     }
 
     private void alterUserInternal(boolean ifExists, OpType opType, 
UserIdentity userIdent, byte[] password,
-            String role, PasswordOptions passwordOptions, boolean isReplay) 
throws DdlException {
+            String role, PasswordOptions passwordOptions, String comment, 
boolean isReplay) throws DdlException {
         writeLock();
         try {
             if (!doesUserExist(userIdent)) {
@@ -1559,6 +1591,9 @@ public class Auth implements Writable {
                 case UNLOCK_ACCOUNT:
                     passwdPolicyManager.unlockUser(userIdent);
                     break;
+                case MODIFY_COMMENT:
+                    modifyComment(userIdent, comment);
+                    break;
                 default:
                     throw new DdlException("Unknown alter user operation type: 
" + opType.name());
             }
@@ -1566,7 +1601,7 @@ public class Auth implements Writable {
                 // For SET_PASSWORD:
                 //      the edit log is wrote in "setPasswordInternal"
                 AlterUserOperationLog log = new AlterUserOperationLog(opType, 
userIdent, password, role,
-                        passwordOptions);
+                        passwordOptions, comment);
                 Env.getCurrentEnv().getEditLog().logAlterUser(log);
             }
         } finally {
@@ -1574,7 +1609,7 @@ public class Auth implements Writable {
         }
     }
 
-    //tmp for current user can only has one role
+    // tmp for current user can only has one role
     private void setRoleToUser(UserIdentity userIdent, String role) throws 
DdlException {
         // 1. check if role exist
         Role newRole = roleManager.getRole(role);
@@ -1586,6 +1621,14 @@ public class Auth implements Writable {
         userRoleManager.addUserRole(userIdent, 
roleManager.getUserDefaultRoleName(userIdent));
     }
 
+    private void modifyComment(UserIdentity userIdent, String comment) throws 
DdlException {
+        User user = userManager.getUserByUserIdentity(userIdent);
+        if (user == null) {
+            throw new DdlException("UserIdentity " + userIdent + " does not 
exist");
+        }
+        user.setComment(comment);
+    }
+
     public Set<String> getAllUser() {
         return userManager.getNameToUsers().keySet();
     }
@@ -1663,15 +1706,15 @@ public class Auth implements Writable {
     private void upgradeToVersion116(UserPrivTable userPrivTable, 
CatalogPrivTable catalogPrivTable,
             DbPrivTable dbPrivTable, TablePrivTable tablePrivTable, 
ResourcePrivTable resourcePrivTable)
             throws AnalysisException, DdlException, PatternMatcherException {
-        //OPERATOR and Admin role not save users,if not inituser,root will do 
not have admin role
+        // OPERATOR and Admin role not save users,if not inituser,root will do 
not have admin role
         initUser();
         for (Entry<String, UserProperty> entry : 
propertyMgr.propertyMap.entrySet()) {
             for (Entry<String, byte[]> userEntry : 
entry.getValue().getWhiteList().getPasswordMap().entrySet()) {
-                //create user
+                // create user
                 User user = userManager
                         
.createUser(UserIdentity.createAnalyzedUserIdentWithDomain(entry.getKey(), 
userEntry.getKey()),
-                                userEntry.getValue(), null, false);
-                //create default role
+                                userEntry.getValue(), null, false, "");
+                // create default role
                 Role defaultRole = 
roleManager.createDefaultRole(user.getUserIdentity());
                 userRoleManager
                         .addUserRole(user.getUserIdentity(), 
defaultRole.getRoleName());
@@ -1680,18 +1723,18 @@ public class Auth implements Writable {
         List<PrivEntry> userPrivTableEntries = userPrivTable.getEntries();
         for (PrivEntry privEntry : userPrivTableEntries) {
             GlobalPrivEntry globalPrivEntry = (GlobalPrivEntry) privEntry;
-            //may repeat with created user from propertyMgr,but no influence
+            // may repeat with created user from propertyMgr,but no influence
             User user = userManager
                     .createUser(globalPrivEntry.userIdentity, 
globalPrivEntry.password, globalPrivEntry.domainUserIdent,
-                            globalPrivEntry.isSetByDomainResolver);
-            //create default role
+                            globalPrivEntry.isSetByDomainResolver, "");
+            // create default role
             Role defaultRole = 
roleManager.createDefaultRole(user.getUserIdentity());
             userRoleManager
                     .addUserRole(user.getUserIdentity(), 
defaultRole.getRoleName());
             if (globalPrivEntry.privSet.isEmpty()) {
                 continue;
             }
-            //grant global auth
+            // grant global auth
             if (globalPrivEntry.privSet.containsResourcePriv()) {
                 roleManager.addOrMergeRole(new 
Role(roleManager.getUserDefaultRoleName(user.getUserIdentity()),
                         ResourcePattern.ALL, 
PrivBitSet.of(Privilege.USAGE_PRIV)), false);
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
index 59a2316dadc..f59cfd699f0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
@@ -78,6 +78,9 @@ public class Role implements Writable, GsonPostProcessable {
 
     @SerializedName(value = "roleName")
     private String roleName;
+
+    @SerializedName(value = "comment")
+    private String comment;
     // Will be persisted
     @SerializedName(value = "tblPatternToPrivs")
     private Map<TablePattern, PrivBitSet> tblPatternToPrivs = 
Maps.newConcurrentMap();
@@ -110,7 +113,12 @@ public class Role implements Writable, GsonPostProcessable 
{
     }
 
     public Role(String roleName) {
+        this(roleName, "");
+    }
+
+    public Role(String roleName, String comment) {
         this.roleName = roleName;
+        this.comment = comment;
     }
 
     public Role(String roleName, TablePattern tablePattern, PrivBitSet privs) 
throws DdlException {
@@ -484,6 +492,14 @@ public class Role implements Writable, GsonPostProcessable 
{
         return workloadGroupPrivTable;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
+    public void setComment(String comment) {
+        this.comment = comment;
+    }
+
     public boolean checkCanEnterCluster(String clusterName) {
         if (checkGlobalPriv(PrivPredicate.ALL)) {
             return true;
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java
index 7f48ed87cfa..e5654b6598c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java
@@ -150,6 +150,7 @@ public class RoleManager implements Writable, 
GsonPostProcessable {
             }
             List<String> info = Lists.newArrayList();
             info.add(role.getRoleName());
+            info.add(role.getComment());
             info.add(Joiner.on(", 
").join(Env.getCurrentEnv().getAuth().getRoleUsers(role.getRoleName())));
             Map<PrivLevel, String> infoMap =
                     Stream.concat(
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/User.java 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/User.java
index afac0767e07..c48d0605c5f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/User.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/User.java
@@ -47,12 +47,14 @@ public class User implements Comparable<User>, Writable, 
GsonPostProcessable {
     protected boolean isAnyHost = false;
     @SerializedName(value = "password")
     private Password password;
+    @SerializedName(value = "comment")
+    private String comment;
 
     public User() {
     }
 
     public User(UserIdentity userIdent, byte[] pwd, boolean setByResolver, 
UserIdentity domainUserIdent,
-            PatternMatcher hostPattern) {
+            PatternMatcher hostPattern, String comment) {
         this.isAnyHost = userIdent.getHost().equals(UserManager.ANY_HOST);
         this.userIdentity = userIdent;
         this.password = new Password(pwd);
@@ -62,6 +64,7 @@ public class User implements Comparable<User>, Writable, 
GsonPostProcessable {
             Preconditions.checkNotNull(domainUserIdent);
             this.domainUserIdentity = domainUserIdent;
         }
+        this.comment = comment;
     }
 
 
@@ -126,6 +129,14 @@ public class User implements Comparable<User>, Writable, 
GsonPostProcessable {
         return password != null && password.getPassword() != null && 
password.getPassword().length != 0;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
+    public void setComment(String comment) {
+        this.comment = comment;
+    }
+
     @Override
     public int compareTo(@NotNull User o) {
         return -userIdentity.getHost().compareTo(o.userIdentity.getHost());
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserManager.java 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserManager.java
index e1c0faa1677..6d677779639 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserManager.java
@@ -54,7 +54,7 @@ public class UserManager implements Writable, 
GsonPostProcessable {
     public static final String ANY_HOST = "%";
     private static final Logger LOG = LogManager.getLogger(UserManager.class);
     // Concurrency control is delegated by Auth, so not concurrentMap
-    //One name may have multiple User,because host can be different
+    // One name may have multiple User,because host can be different
     @SerializedName(value = "nameToUsers")
     private Map<String, List<User>> nameToUsers = Maps.newHashMap();
 
@@ -181,7 +181,8 @@ public class UserManager implements Writable, 
GsonPostProcessable {
 
     }
 
-    public User createUser(UserIdentity userIdent, byte[] pwd, UserIdentity 
domainUserIdent, boolean setByResolver)
+    public User createUser(UserIdentity userIdent, byte[] pwd, UserIdentity 
domainUserIdent, boolean setByResolver,
+            String comment)
             throws PatternMatcherException {
         if (userIdentityExist(userIdent, true)) {
             User userByUserIdentity = getUserByUserIdentity(userIdent);
@@ -192,13 +193,14 @@ public class UserManager implements Writable, 
GsonPostProcessable {
                 return userByUserIdentity;
             }
             userByUserIdentity.setPassword(pwd);
+            userByUserIdentity.setComment(comment);
             userByUserIdentity.setSetByDomainResolver(setByResolver);
             return userByUserIdentity;
         }
 
         PatternMatcher hostPattern = PatternMatcher
                 .createMysqlPattern(userIdent.getHost(), 
CaseSensibility.HOST.getCaseSensibility());
-        User user = new User(userIdent, pwd, setByResolver, domainUserIdent, 
hostPattern);
+        User user = new User(userIdent, pwd, setByResolver, domainUserIdent, 
hostPattern, comment);
         List<User> nameToLists = nameToUsers.get(userIdent.getQualifiedUser());
         if (CollectionUtils.isEmpty(nameToLists)) {
             nameToLists = Lists.newArrayList(user);
@@ -286,7 +288,7 @@ public class UserManager implements Writable, 
GsonPostProcessable {
                     byte[] password = domainUser.getPassword().getPassword();
                     Preconditions.checkNotNull(password, entry.getKey());
                     try {
-                        createUser(userIdent, password, 
domainUser.getUserIdentity(), true);
+                        createUser(userIdent, password, 
domainUser.getUserIdentity(), true, "");
                     } catch (PatternMatcherException e) {
                         LOG.info("failed to create user for user ident: {}, 
{}", userIdent, e.getMessage());
                     }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/persist/AlterUserOperationLog.java 
b/fe/fe-core/src/main/java/org/apache/doris/persist/AlterUserOperationLog.java
index 5d4ea006885..472cda7874e 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/persist/AlterUserOperationLog.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/persist/AlterUserOperationLog.java
@@ -43,13 +43,17 @@ public class AlterUserOperationLog implements Writable {
     @SerializedName(value = "op")
     private AlterUserStmt.OpType op;
 
+    @SerializedName(value = "comment")
+    private String comment;
+
     public AlterUserOperationLog(OpType opType, UserIdentity userIdent, byte[] 
password,
-            String role, PasswordOptions passwordOptions) {
+            String role, PasswordOptions passwordOptions, String comment) {
         this.op = opType;
         this.userIdent = userIdent;
         this.password = password;
         this.role = role;
         this.passwordOptions = passwordOptions;
+        this.comment = comment;
     }
 
     public OpType getOp() {
@@ -72,6 +76,10 @@ public class AlterUserOperationLog implements Writable {
         return passwordOptions;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     @Override
     public void write(DataOutput out) throws IOException {
         Text.writeString(out, GsonUtils.GSON.toJson(this));
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java 
b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
index d21012dd658..ffae17c6b1c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
@@ -479,6 +479,11 @@ public class EditLog {
                     env.getAuth().replayCreateRole(privInfo);
                     break;
                 }
+                case OperationType.OP_ALTER_ROLE: {
+                    PrivInfo privInfo = (PrivInfo) journal.getData();
+                    env.getAuth().replayAlterRole(privInfo);
+                    break;
+                }
                 case OperationType.OP_DROP_ROLE: {
                     PrivInfo privInfo = (PrivInfo) journal.getData();
                     env.getAuth().replayDropRole(privInfo);
@@ -1504,6 +1509,10 @@ public class EditLog {
         logEdit(OperationType.OP_CREATE_ROLE, info);
     }
 
+    public void logAlterRole(PrivInfo info) {
+        logEdit(OperationType.OP_ALTER_ROLE, info);
+    }
+
     public void logDropRole(PrivInfo info) {
         logEdit(OperationType.OP_DROP_ROLE, info);
     }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java 
b/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
index c88dd029502..f5c71e58b72 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
@@ -382,6 +382,8 @@ public class OperationType {
 
     public static final short OP_DROP_PLSQL_PACKAGE = 474;
 
+    public static final short OP_ALTER_ROLE = 475;
+
     /**
      * Get opcode name by op code.
      **/
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java 
b/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java
index 6cb11f8cb2c..ca5a35de0f5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java
@@ -56,6 +56,8 @@ public class PrivInfo implements Writable, 
GsonPostProcessable {
     private byte[] passwd;
     @SerializedName(value = "role")
     private String role;
+    @SerializedName(value = "comment")
+    private String comment;
     @SerializedName(value = "colPrivileges")
     private Map<ColPrivilegeKey, Set<String>> colPrivileges;
     @SerializedName(value = "passwordOptions")
@@ -71,6 +73,11 @@ public class PrivInfo implements Writable, 
GsonPostProcessable {
     // For create user/set password/create role/drop role
     public PrivInfo(UserIdentity userIdent, PrivBitSet privs, byte[] passwd, 
String role,
             PasswordOptions passwordOptions) {
+        this(userIdent, privs, passwd, role, passwordOptions, null);
+    }
+
+    public PrivInfo(UserIdentity userIdent, PrivBitSet privs, byte[] passwd, 
String role,
+            PasswordOptions passwordOptions, String comment) {
         this.userIdent = userIdent;
         this.tblPattern = null;
         this.resourcePattern = null;
@@ -78,6 +85,12 @@ public class PrivInfo implements Writable, 
GsonPostProcessable {
         this.passwd = passwd;
         this.role = role;
         this.passwordOptions = passwordOptions;
+        this.comment = comment;
+    }
+
+    public PrivInfo(String role, String comment) {
+        this.role = role;
+        this.comment = comment;
     }
 
     // For grant/revoke
@@ -150,6 +163,10 @@ public class PrivInfo implements Writable, 
GsonPostProcessable {
         return role;
     }
 
+    public String getComment() {
+        return comment;
+    }
+
     public PasswordOptions getPasswordOptions() {
         return passwordOptions == null ? PasswordOptions.UNSET_OPTION : 
passwordOptions;
     }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java 
b/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
index ef6a47de691..35632c5e430 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
@@ -41,6 +41,7 @@ import org.apache.doris.analysis.AlterJobStatusStmt;
 import org.apache.doris.analysis.AlterPolicyStmt;
 import org.apache.doris.analysis.AlterRepositoryStmt;
 import org.apache.doris.analysis.AlterResourceStmt;
+import org.apache.doris.analysis.AlterRoleStmt;
 import org.apache.doris.analysis.AlterRoutineLoadStmt;
 import org.apache.doris.analysis.AlterSqlBlockRuleStmt;
 import org.apache.doris.analysis.AlterSystemStmt;
@@ -228,6 +229,8 @@ public class DdlExecutor {
             env.getAuth().revoke(stmt);
         } else if (ddlStmt instanceof CreateRoleStmt) {
             env.getAuth().createRole((CreateRoleStmt) ddlStmt);
+        } else if (ddlStmt instanceof AlterRoleStmt) {
+            env.getAuth().alterRole((AlterRoleStmt) ddlStmt);
         } else if (ddlStmt instanceof DropRoleStmt) {
             env.getAuth().dropRole((DropRoleStmt) ddlStmt);
         } else if (ddlStmt instanceof SetUserPropertyStmt) {
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
index 0bbc84b5b1a..bf52ea76225 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
@@ -888,7 +888,7 @@ public class AuthTest {
         }
 
         // 24.1 create role again with IF NOT EXISTS
-        roleStmt = new CreateRoleStmt(true, "role1");
+        roleStmt = new CreateRoleStmt(true, "role1", "");
         try {
             roleStmt.analyze(analyzer);
         } catch (UserException e1) {
@@ -904,7 +904,7 @@ public class AuthTest {
         }
 
         // 24.2 create role again without IF NOT EXISTS
-        roleStmt = new CreateRoleStmt(false, "role1");
+        roleStmt = new CreateRoleStmt(false, "role1", "");
         try {
             roleStmt.analyze(analyzer);
         } catch (UserException e1) {
diff --git a/regression-test/suites/account_p0/test_account.groovy 
b/regression-test/suites/account_p0/test_account.groovy
index 1eb07e259cf..accea7bb8c5 100644
--- a/regression-test/suites/account_p0/test_account.groovy
+++ b/regression-test/suites/account_p0/test_account.groovy
@@ -14,12 +14,27 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-suite("test_account") {
-    // todo: test account management, such as role, user, grant, revoke ...
-    sql "show roles"
 
+import org.junit.Assert;
+
+suite("test_account") {
+    // test comment
+    def user = "test_account_comment_user";
+    sql """drop user if exists ${user}"""
+    // create user with comment
+    sql """create user ${user} comment 
'test_account_comment_user_comment_create'"""
+    def user_create = sql "show grants for ${user}"
+    logger.info("user_create: " + user_create.toString())
+    
assertTrue(user_create.toString().contains("test_account_comment_user_comment_create"))
+    // alter user comment
+    sql """alter user ${user} comment 
'test_account_comment_user_comment_alter'"""
+    def user_alter = sql "show grants for ${user}"
+    logger.info("user_alter: " + user_alter.toString())
+    
assertTrue(user_alter.toString().contains("test_account_comment_user_comment_alter"))
+    // drop user
+    sql """drop user if exists ${user}"""
     try {
-        sql "show grants for 'non_existent_user_1'"
+        sql "show grants for ${user}"
         fail()
     } catch (Exception e) {
         log.info(e.getMessage())
diff --git a/regression-test/suites/account_p0/test_revoke_role.groovy 
b/regression-test/suites/account_p0/test_revoke_role.groovy
index 3e244232973..5aa9f77206a 100644
--- a/regression-test/suites/account_p0/test_revoke_role.groovy
+++ b/regression-test/suites/account_p0/test_revoke_role.groovy
@@ -35,12 +35,12 @@ suite("test_revoke_role", "account") {
 
     def result = sql """ SHOW GRANTS FOR ${user} """
     assertEquals(result.size(), 1)
-    assertTrue(result[0][5].contains("internal.${dbName}: Select_priv"))
+    assertTrue(result[0][6].contains("internal.${dbName}: Select_priv"))
 
     sql """REVOKE '${role}' from ${user}"""
     result = sql """ SHOW GRANTS FOR ${user} """
     assertEquals(result.size(), 1)
-    assertFalse(result[0][5].contains("internal.${dbName}: Select_priv"))
+    assertFalse(result[0][6].contains("internal.${dbName}: Select_priv"))
 
     sql """DROP USER ${user}"""
     sql """DROP ROLE ${role}"""
diff --git a/regression-test/suites/account_p0/test_role.groovy 
b/regression-test/suites/account_p0/test_role.groovy
index e3b227ea004..6d88238ebf5 100644
--- a/regression-test/suites/account_p0/test_role.groovy
+++ b/regression-test/suites/account_p0/test_role.groovy
@@ -15,6 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+import org.junit.Assert;
+
 suite("test_role", "account") {
     def role= 'account_role_test'
     def user = 'acount_role_user_test'
@@ -44,5 +46,22 @@ suite("test_role", "account") {
     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
+    sql """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"))
 }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to