XN137 commented on PR #2290: URL: https://github.com/apache/polaris/pull/2290#issuecomment-3184684975
thanks for the feedback, since the other PR is somewhat merge-ready i will wait for that and rebase this one afterwards. > Also rename the filter/transformer variations of listEntities within the BasePersistence yeah we can use the same `loadEntities` name in `BasePersistence` imo > Pull filter/transformer evaluation out of BasePersistence into MetaStoreManager i havent investigated this in detail yet but my current guess is that the filter needs to remain "pushed down" in order for pagination to work correctly, but will double check that later. > Reconsider whether we actually need a Function<PolarisBaseEntity, T> generic type return value afaict the transformer is used heavily by all callers of `load(All)Entities` in this PR i.e. to turn base entities into their more specific type (for example via `CatalogRoleEntity::of`). so i dont see that going away. unless you mean that this transformation can also happen on the "outside" ? this might be possible however one idea I had was that one might not need both the filter and the transformer but we could let the transformer return `null` for entities the caller does not need (or that cant be converted to the more specific type). but even then the transformer needs to remain "Pushed down" to work well with pagination most likely. then again, such a transformation -- 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: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
