sungwy commented on code in PR #3940: URL: https://github.com/apache/polaris/pull/3940#discussion_r2893087335
########## polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java: ########## @@ -57,11 +57,11 @@ public class PolarisResolutionManifest implements PolarisResolutionManifestCatal private final Resolver primaryResolver; private final PolarisDiagnostics diagnostics; - private final Map<Object, Integer> pathLookup = new HashMap<>(); + private final Map<ResolvedPathKey, Integer> pathLookup = new HashMap<>(); Review Comment: Hi @dimas-b - funny you mention this, because I was thinking just the same thing. But I fell short of proposing it because: 1. It wasn't absolutely necessary for the `PolarisAuthorizer` SPI refactoring needs 2. I wasn't really sure if there was some hidden sauce reason why we were doing the `Integer` based lookup... Since that's an underlying implementation detail, how about we leave that out of scope of this PR and revisit it after a careful, dedicated review? cc @dennishuo -- 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]
