dimas-b commented on code in PR #3940: URL: https://github.com/apache/polaris/pull/3940#discussion_r2892965291
########## 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: I believe this map is generally used only to allow caller to retrieved resolved entities for some "seed" parameters. Some time ago I tried refactoring this code in #499 with the idea that callers would get back "handle" objects for "seed" parameter they put into the resolver flow. That PR did not merge, but I still wanted to mention it again. I wonder what you think about it. This is just an idea for consideration. No pressure 😉 I believe callers never need to use arbitrary lookup keys (at least they did not do that at the time of that old PR), they always obtain the resolved object for some kind of input parameter they already have, so the "handle" objects can usually be carried around as local variables. -- 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]
