[GitHub] [incubator-doris] morningman commented on a diff in pull request #8849: Add [IF EXISTS] support to account management SQL

[GitBox <gi...@apache.org>]

morningman commented on code in PR #8849:
URL: https://github.com/apache/incubator-doris/pull/8849#discussion_r844608265


##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java:
##########
@@ -646,22 +650,33 @@ private void grantPrivsByRole(UserIdentity userIdent, PaloRole role) throws DdlE
 
     // drop user
     public void dropUser(DropUserStmt stmt) throws DdlException {
-        dropUserInternal(stmt.getUserIdentity(), false);
+        dropUserInternal(stmt.getUserIdentity(), stmt.isSetIfExists(), false);
     }
 
-    public void replayDropUser(UserIdentity userIdent) {
-        dropUserInternal(userIdent, true);
+    public void replayDropUser(UserIdentity userIdent) throws DdlException {
+        dropUserInternal(userIdent, false, true);
     }
 
-    public void replayOldDropUser(String userName) {
+    public void replayOldDropUser(String userName) throws DdlException {
         UserIdentity userIdentity = new UserIdentity(userName, "%");
         userIdentity.setIsAnalyzed();
-        dropUserInternal(userIdentity, true /* is replay */);
+        dropUserInternal(userIdentity, false /* ignore if non exists */, true /* is replay */);
     }
 
-    private void dropUserInternal(UserIdentity userIdent, boolean isReplay) {
+    private void dropUserInternal(UserIdentity userIdent, boolean ignoreIfNonExists, boolean isReplay) throws DdlException {
         writeLock();
         try {
+            // check if user exists
+            if (!doesUserExist(userIdent)) {
+                if (ignoreIfNonExists) {
+                    LOG.info("user non exists, ignored to drop user: {}, is replay: {}",

Review Comment:
   ```suggestion
                       LOG.debug("user non exists, ignored to drop user: {}, is replay: {}",
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java:
##########
@@ -1076,20 +1096,25 @@ private void createRoleInternal(String role, boolean isReplay) throws DdlExcepti
 
     // drop role
     public void dropRole(DropRoleStmt stmt) throws DdlException {
-        dropRoleInternal(stmt.getQualifiedRole(), false);
+        dropRoleInternal(stmt.getQualifiedRole(), stmt.isSetIfExists(), false);
     }
 
     public void replayDropRole(PrivInfo info) {
         try {
-            dropRoleInternal(info.getRole(), true);
+            dropRoleInternal(info.getRole(), false, true);
         } catch (DdlException e) {
             LOG.error("should not happened", e);
         }
     }
 
-    private void dropRoleInternal(String role, boolean isReplay) throws DdlException {
+    private void dropRoleInternal(String role, boolean ignoreIfNonExists, boolean isReplay) throws DdlException {
         writeLock();
         try {
+            if (ignoreIfNonExists && roleManager.getRole(role) == null) {
+                LOG.info("role non exists, ignored to drop role: {}, is replay: {}", role, isReplay);

Review Comment:
   ```suggestion
                   LOG.debug("role non exists, ignored to drop role: {}, is replay: {}", role, isReplay);
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java:
##########
@@ -1047,21 +1062,26 @@ public void replaySetLdapPassword(LdapInfo info) {
 
     // create role
     public void createRole(CreateRoleStmt stmt) throws DdlException {
-        createRoleInternal(stmt.getQualifiedRole(), false);
+        createRoleInternal(stmt.getQualifiedRole(), stmt.isSetIfNotExists(), false);
     }
 
     public void replayCreateRole(PrivInfo info) {
         try {
-            createRoleInternal(info.getRole(), true);
+            createRoleInternal(info.getRole(), false, true);
         } catch (DdlException e) {
             LOG.error("should not happened", e);
         }
     }
 
-    private void createRoleInternal(String role, boolean isReplay) throws DdlException {
+    private void createRoleInternal(String role, boolean ignoreIfExists, boolean isReplay) throws DdlException {
         PaloRole emptyPrivsRole = new PaloRole(role);
         writeLock();
         try {
+            if (ignoreIfExists && roleManager.getRole(role) != null) {
+                LOG.info("role exists, ignored to create role: {}, is replay: {}", role, isReplay);

Review Comment:
   ```suggestion
                   LOG.debug("role exists, ignored to create role: {}, is replay: {}", role, isReplay);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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