This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new c0497eb0cf9 [improvement](fe) Short-circuit catalog/db/table privilege
check when global privilege is granted (#63838)
c0497eb0cf9 is described below
commit c0497eb0cf9fa5a03ea68b7c0619461f3eadd3a1
Author: heguanhui <[email protected]>
AuthorDate: Fri Jun 12 22:25:24 2026 +0800
[improvement](fe) Short-circuit catalog/db/table privilege check when
global privilege is granted (#63838)
## Summary
Short-circuit `checkCtlPriv`/`checkDbPriv`/`checkTblPriv` in
`CatalogAccessController` when `hasGlobal=true`, avoiding unnecessary
privilege lookups that become a performance bottleneck with large
numbers of privileges.
## What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary: In `CatalogAccessController`, the
`checkCtlPriv`/`checkDbPriv`/`checkTblPriv` methods with `hasGlobal`
parameter always invoke the underlying privilege check (e.g.
`checkCtlPriv(currentUser, ctl, wanted)`) even when `hasGlobal` is true.
Since `hasGlobal=true` means the user already has global-level
privilege, the result of the specific-level check is irrelevant — the
method will return true regardless. This causes unnecessary privilege
lookups, which become a performance bottleneck when there are a large
number of privileges configured.
Before:
```java
default boolean checkCtlPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, PrivPredicate wanted) {
boolean res = checkCtlPriv(currentUser, ctl, wanted);
return hasGlobal || res;
}
```
After:
```java
default boolean checkCtlPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, PrivPredicate wanted) {
if (hasGlobal) {
return true;
}
return checkCtlPriv(currentUser, ctl, wanted);
}
```
The same pattern is applied to `checkDbPriv` and `checkTblPriv`.
## Release note
Optimized privilege checking to short-circuit when global privilege is
already granted, avoiding unnecessary catalog/db/table-level privilege
lookups and improving performance in environments with many privileges.
---
.../mysql/privilege/CatalogAccessController.java | 27 ++-
.../privilege/CatalogAccessControllerTest.java | 258 +++++++++++++++++++++
2 files changed, 273 insertions(+), 12 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/CatalogAccessController.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/CatalogAccessController.java
index 0538e52c287..df7605256dd 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/CatalogAccessController.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/CatalogAccessController.java
@@ -28,8 +28,10 @@ import java.util.Set;
public interface CatalogAccessController {
// ==== Catalog ====
default boolean checkCtlPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, PrivPredicate wanted) {
- boolean res = checkCtlPriv(currentUser, ctl, wanted);
- return hasGlobal || res;
+ if (hasGlobal) {
+ return true;
+ }
+ return checkCtlPriv(currentUser, ctl, wanted);
}
// ==== Global ====
@@ -41,8 +43,10 @@ public interface CatalogAccessController {
// ==== Database ====
default boolean checkDbPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, String db,
PrivPredicate wanted) {
- boolean res = checkDbPriv(currentUser, ctl, db, wanted);
- return hasGlobal || res;
+ if (hasGlobal) {
+ return true;
+ }
+ return checkDbPriv(currentUser, ctl, db, wanted);
}
boolean checkDbPriv(UserIdentity currentUser, String ctl, String db,
PrivPredicate wanted);
@@ -50,8 +54,10 @@ public interface CatalogAccessController {
// ==== Table ====
default boolean checkTblPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, String db, String tbl,
PrivPredicate wanted) {
- boolean res = checkTblPriv(currentUser, ctl, db, tbl, wanted);
- return hasGlobal || res;
+ if (hasGlobal) {
+ return true;
+ }
+ return checkTblPriv(currentUser, ctl, db, tbl, wanted);
}
boolean checkTblPriv(UserIdentity currentUser, String ctl, String db,
String tbl, PrivPredicate wanted);
@@ -59,13 +65,10 @@ public interface CatalogAccessController {
// ==== Column ====
default void checkColsPriv(boolean hasGlobal, UserIdentity currentUser,
String ctl, String db, String tbl,
Set<String> cols, PrivPredicate wanted) throws
AuthorizationException {
- try {
- checkColsPriv(currentUser, ctl, db, tbl, cols, wanted);
- } catch (AuthorizationException e) {
- if (!hasGlobal) {
- throw e;
- }
+ if (hasGlobal) {
+ return;
}
+ checkColsPriv(currentUser, ctl, db, tbl, cols, wanted);
}
// ==== Resource ====
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/CatalogAccessControllerTest.java
b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/CatalogAccessControllerTest.java
new file mode 100644
index 00000000000..163518b33a5
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/CatalogAccessControllerTest.java
@@ -0,0 +1,258 @@
+// 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.mysql.privilege;
+
+import org.apache.doris.analysis.ResourceTypeEnum;
+import org.apache.doris.analysis.UserIdentity;
+import org.apache.doris.common.AuthorizationException;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public class CatalogAccessControllerTest {
+
+ private static class StubAccessController implements
CatalogAccessController {
+ final AtomicBoolean ctlPrivCalled = new AtomicBoolean(false);
+ final AtomicBoolean dbPrivCalled = new AtomicBoolean(false);
+ final AtomicBoolean tblPrivCalled = new AtomicBoolean(false);
+ final AtomicBoolean colsPrivCalled = new AtomicBoolean(false);
+
+ private final boolean ctlResult;
+ private final boolean dbResult;
+ private final boolean tblResult;
+ private final boolean colsResult;
+
+ StubAccessController() {
+ this(false, false, false, false);
+ }
+
+ StubAccessController(boolean ctlResult, boolean dbResult, boolean
tblResult) {
+ this(ctlResult, dbResult, tblResult, false);
+ }
+
+ StubAccessController(boolean ctlResult, boolean dbResult, boolean
tblResult, boolean colsResult) {
+ this.ctlResult = ctlResult;
+ this.dbResult = dbResult;
+ this.tblResult = tblResult;
+ this.colsResult = colsResult;
+ }
+
+ @Override
+ public boolean checkGlobalPriv(UserIdentity currentUser, PrivPredicate
wanted) {
+ return false;
+ }
+
+ @Override
+ public boolean checkCtlPriv(UserIdentity currentUser, String ctl,
PrivPredicate wanted) {
+ ctlPrivCalled.set(true);
+ return ctlResult;
+ }
+
+ @Override
+ public boolean checkDbPriv(UserIdentity currentUser, String ctl,
String db, PrivPredicate wanted) {
+ dbPrivCalled.set(true);
+ return dbResult;
+ }
+
+ @Override
+ public boolean checkTblPriv(UserIdentity currentUser, String ctl,
String db, String tbl, PrivPredicate wanted) {
+ tblPrivCalled.set(true);
+ return tblResult;
+ }
+
+ @Override
+ public void checkColsPriv(UserIdentity currentUser, String ctl, String
db, String tbl,
+ Set<String> cols, PrivPredicate wanted) throws
AuthorizationException {
+ colsPrivCalled.set(true);
+ if (!colsResult) {
+ throw new AuthorizationException("denied");
+ }
+ }
+
+ @Override
+ public boolean checkResourcePriv(UserIdentity currentUser, String
resourceName, PrivPredicate wanted) {
+ return false;
+ }
+
+ @Override
+ public boolean checkWorkloadGroupPriv(UserIdentity currentUser, String
workloadGroupName,
+ PrivPredicate wanted) {
+ return false;
+ }
+
+ @Override
+ public boolean checkCloudPriv(UserIdentity currentUser, String
cloudName,
+ PrivPredicate wanted, ResourceTypeEnum type) {
+ return false;
+ }
+
+ @Override
+ public boolean checkStorageVaultPriv(UserIdentity currentUser, String
storageVaultName,
+ PrivPredicate wanted) {
+ return false;
+ }
+
+ @Override
+ public Optional<DataMaskPolicy> evalDataMaskPolicy(UserIdentity
currentUser, String ctl, String db,
+ String tbl, String col) {
+ return Optional.empty();
+ }
+
+ @Override
+ public List<? extends RowFilterPolicy>
evalRowFilterPolicies(UserIdentity currentUser, String ctl,
+ String db, String tbl) {
+ return ImmutableList.of();
+ }
+ }
+
+ @Test
+ public void testCheckCtlPrivShortCircuitOnHasGlobal() {
+ StubAccessController controller = new StubAccessController();
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkCtlPriv(true, user, "ctl",
PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertFalse(controller.ctlPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckCtlPrivFallsThroughWithoutHasGlobal() {
+ StubAccessController controller = new StubAccessController(true,
false, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkCtlPriv(false, user, "ctl",
PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertTrue(controller.ctlPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckCtlPrivFallsThroughAndReturnsFalse() {
+ StubAccessController controller = new StubAccessController(false,
false, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkCtlPriv(false, user, "ctl",
PrivPredicate.SELECT);
+
+ Assert.assertFalse(result);
+ Assert.assertTrue(controller.ctlPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckDbPrivShortCircuitOnHasGlobal() {
+ StubAccessController controller = new StubAccessController();
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkDbPriv(true, user, "ctl", "db",
PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertFalse(controller.dbPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckDbPrivFallsThroughWithoutHasGlobal() {
+ StubAccessController controller = new StubAccessController(false,
true, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkDbPriv(false, user, "ctl", "db",
PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertTrue(controller.dbPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckDbPrivFallsThroughAndReturnsFalse() {
+ StubAccessController controller = new StubAccessController(false,
false, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkDbPriv(false, user, "ctl", "db",
PrivPredicate.SELECT);
+
+ Assert.assertFalse(result);
+ Assert.assertTrue(controller.dbPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckTblPrivShortCircuitOnHasGlobal() {
+ StubAccessController controller = new StubAccessController();
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkTblPriv(true, user, "ctl", "db",
"tbl", PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertFalse(controller.tblPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckTblPrivFallsThroughWithoutHasGlobal() {
+ StubAccessController controller = new StubAccessController(false,
false, true);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkTblPriv(false, user, "ctl", "db",
"tbl", PrivPredicate.SELECT);
+
+ Assert.assertTrue(result);
+ Assert.assertTrue(controller.tblPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckTblPrivFallsThroughAndReturnsFalse() {
+ StubAccessController controller = new StubAccessController(false,
false, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ boolean result = controller.checkTblPriv(false, user, "ctl", "db",
"tbl", PrivPredicate.SELECT);
+
+ Assert.assertFalse(result);
+ Assert.assertTrue(controller.tblPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckColsPrivShortCircuitOnHasGlobal() throws
AuthorizationException {
+ StubAccessController controller = new StubAccessController();
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ controller.checkColsPriv(true, user, "ctl", "db", "tbl",
ImmutableSet.of("col1"), PrivPredicate.SELECT);
+
+ Assert.assertFalse(controller.colsPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckColsPrivFallsThroughWithoutHasGlobal() throws
AuthorizationException {
+ StubAccessController controller = new StubAccessController(false,
false, false, true);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ controller.checkColsPriv(false, user, "ctl", "db", "tbl",
ImmutableSet.of("col1"), PrivPredicate.SELECT);
+
+ Assert.assertTrue(controller.colsPrivCalled.get());
+ }
+
+ @Test
+ public void testCheckColsPrivFallsThroughAndThrows() {
+ StubAccessController controller = new StubAccessController(false,
false, false, false);
+ UserIdentity user =
UserIdentity.createAnalyzedUserIdentWithIp("test_user", "%");
+
+ Assert.assertThrows(AuthorizationException.class, () ->
+ controller.checkColsPriv(false, user, "ctl", "db", "tbl",
ImmutableSet.of("col1"), PrivPredicate.SELECT));
+ Assert.assertTrue(controller.colsPrivCalled.get());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
