sungwy commented on code in PR #3681:
URL: https://github.com/apache/polaris/pull/3681#discussion_r2791114611
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java:
##########
@@ -908,4 +913,75 @@ public boolean hasTransitivePrivilege(
resolvedPath);
return false;
}
+
+ @Override
+ public void preAuthorize(
+ @Nonnull AuthorizationCallContext ctx, @Nonnull AuthorizationRequest
request) {
+ PolarisResolutionManifest manifest = ctx.getResolutionManifest();
+ if (manifest.hasResolution()) {
+ return;
+ }
Review Comment:
> Would it make sense to fold this into resolveSelections()
I think that makes sense, and there are two possible implementation
approaches.
1. Introduce an idempotent resolution mechanism for each item, allowing the
caller to iteratively resolve additional or different selections, as you
suggested.
2. If the selection set is unchanged, return cached resolution results
without re-resolving; if a different selection set is requested, fail fast with
an error.
Option (2) is safe and preserves semantics similar to the current behavior,
where only a single resolveAll() is allowed.
In my view, option (1) is only safe if one of the following holds:
- resolving different selections across different snapshots of the
persistence layer is acceptable, or
- the resolver or persistence layer already operates over a consistent
snapshot.
I’d need a deeper understanding of the persistence model and its consistency
guarantees before taking a strong position on option (1)...
--
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]
