morningman commented on code in PR #23022:
URL: https://github.com/apache/doris/pull/23022#discussion_r1299387720
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java:
##########
@@ -125,18 +125,14 @@ public Optional<Expression> getFilter(LogicalRelation
logicalRelation, ConnectCo
PolicyMgr policyMgr = connectContext.getEnv().getPolicyMgr();
UserIdentity currentUserIdentity =
connectContext.getCurrentUserIdentity();
- String user = connectContext.getQualifiedUser();
if (currentUserIdentity.isRootUser() ||
currentUserIdentity.isAdminUser()) {
return Optional.empty();
}
- if (!policyMgr.existPolicy(user)) {
- return Optional.empty();
- }
CatalogRelation catalogRelation = (CatalogRelation) logicalRelation;
long dbId = catalogRelation.getDatabase().getId();
long tableId = catalogRelation.getTable().getId();
- List<RowPolicy> policies = policyMgr.getMatchRowPolicy(dbId, tableId,
currentUserIdentity);
+ List<RowPolicy> policies = policyMgr.getUserPolicys(dbId, tableId,
currentUserIdentity);
Review Comment:
```suggestion
List<RowPolicy> policies = policyMgr.getUserPolicies(dbId, tableId,
currentUserIdentity);
```
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -290,51 +277,86 @@ private void unprotectedDrop(DropPolicyLog log) {
if (policy instanceof StoragePolicy) {
((StoragePolicy) policy).removeResourceReference();
}
+ if (policy instanceof RowPolicy) {
+ dropTablePolicys((RowPolicy) policy);
+ }
return true;
}
return false;
});
typeToPolicyMap.put(log.getType(), policies);
- updateMergeTablePolicyMap();
}
/**
* Match row policy and return it.
**/
- public RowPolicy getMatchTablePolicy(long dbId, long tableId, String user)
{
+ public RowPolicy getMatchTablePolicy(long dbId, long tableId, UserIdentity
user) {
+ List<RowPolicy> res = getUserPolicys(dbId, tableId, user);
+ if (CollectionUtils.isEmpty(res)) {
+ return null;
+ }
+ return mergeRowPolicys(res);
+ }
+
+ public List<RowPolicy> getUserPolicys(long dbId, long tableId,
UserIdentity user) {
+ Set<String> roles =
Env.getCurrentEnv().getAccessManager().getAuth().getRolesByUserWithLdap(user).stream()
+ .map(role ->
ClusterNamespace.getNameFromFullName(role.getRoleName())).collect(Collectors.toSet());
+ List<RowPolicy> res = Lists.newArrayList();
readLock();
try {
- if (!dbIdToMergeTablePolicyMap.containsKey(dbId)) {
- return null;
+ if (!tablePolicys.containsKey(dbId) ||
!tablePolicys.get(dbId).containsKey(tableId)) {
+ return res;
}
- String key = Joiner.on("-").join(tableId,
PolicyTypeEnum.ROW.name(), user);
- if (!dbIdToMergeTablePolicyMap.get(dbId).containsKey(key)) {
- return null;
+ List<RowPolicy> policys = tablePolicys.get(dbId).get(tableId);
+ if (policys.size() == 0) {
Review Comment:
This `if` can be removed
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -382,84 +407,48 @@ public ShowResultSet showPolicy(ShowPolicyStmt showStmt)
throws AnalysisExceptio
}
}
+ private void addTablePolicys(RowPolicy policy) {
+ if (policy.getUser() != null) {
+ policy.getUser().setIsAnalyzed();
+ }
+ List<RowPolicy> policys = getOrCreateTblPolicys(policy.getDbId(),
policy.getTableId());
Review Comment:
Check all `policys` spell
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -382,84 +407,48 @@ public ShowResultSet showPolicy(ShowPolicyStmt showStmt)
throws AnalysisExceptio
}
}
+ private void addTablePolicys(RowPolicy policy) {
Review Comment:
```suggestion
private void addTablePoliciess(RowPolicy policy) {
```
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -290,51 +277,86 @@ private void unprotectedDrop(DropPolicyLog log) {
if (policy instanceof StoragePolicy) {
((StoragePolicy) policy).removeResourceReference();
}
+ if (policy instanceof RowPolicy) {
+ dropTablePolicys((RowPolicy) policy);
+ }
return true;
}
return false;
});
typeToPolicyMap.put(log.getType(), policies);
- updateMergeTablePolicyMap();
}
/**
* Match row policy and return it.
**/
- public RowPolicy getMatchTablePolicy(long dbId, long tableId, String user)
{
+ public RowPolicy getMatchTablePolicy(long dbId, long tableId, UserIdentity
user) {
+ List<RowPolicy> res = getUserPolicys(dbId, tableId, user);
+ if (CollectionUtils.isEmpty(res)) {
+ return null;
+ }
+ return mergeRowPolicys(res);
+ }
+
+ public List<RowPolicy> getUserPolicys(long dbId, long tableId,
UserIdentity user) {
+ Set<String> roles =
Env.getCurrentEnv().getAccessManager().getAuth().getRolesByUserWithLdap(user).stream()
+ .map(role ->
ClusterNamespace.getNameFromFullName(role.getRoleName())).collect(Collectors.toSet());
+ List<RowPolicy> res = Lists.newArrayList();
readLock();
try {
- if (!dbIdToMergeTablePolicyMap.containsKey(dbId)) {
- return null;
+ if (!tablePolicys.containsKey(dbId) ||
!tablePolicys.get(dbId).containsKey(tableId)) {
Review Comment:
We can first check this outside the lock, to avoid getting `roles` all the
time
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -382,84 +407,48 @@ public ShowResultSet showPolicy(ShowPolicyStmt showStmt)
throws AnalysisExceptio
}
}
+ private void addTablePolicys(RowPolicy policy) {
+ if (policy.getUser() != null) {
+ policy.getUser().setIsAnalyzed();
+ }
+ List<RowPolicy> policys = getOrCreateTblPolicys(policy.getDbId(),
policy.getTableId());
Review Comment:
```suggestion
List<RowPolicy> policies = getOrCreateTblPolicies(policy.getDbId(),
policy.getTableId());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
