XJDKC commented on PR #1899: URL: https://github.com/apache/polaris/pull/1899#issuecomment-2978053793
> For better or worse, we currently use entities both for the persisted state of an object and for an in-memory representation of the same. I'm still a fan of separating this out (something like the DAO proposal from months back) but that a big refactor that I'm not sure I'd want to block SigV4. > > @snazy perhaps we can view this as a step in the right direction as it encourages us to treat entities as immutable and to use discrete "transformations" to generate new entities instead of trying to modify them in place or to construct lots of new entities in an adhoc way throughout the code base. Eventually, entities should be truly immutable and this may become the canonical way to transform an entity into a new one. > > I am, however, a little concerned about how heavyweight this process could become... if I just want to update an entity's version today, I can call a method like `setGrantRecordsVersion`. After this change, it looks like the expectation is that I have a transformation engine, and then I construct and execute a transformation to accomplish this change? Yes, the Entity Transformation System I'm adding won't update the entity in place, it creates a copy and applies the modifications there. What I want to point out is that, this system isn't meant to replace the current set methods used in the persistence layer. Its purpose is to provide hooks for applying transformations at specific points in the entity lifecycle. It's admittedly a bit heavyweight, since each transformer creates a new copy of the entity. But without a builder-style abstraction (like what we have for generated model classes), we can't apply the transformation efficiently. Given that we may eventually refactor this area to introduce a proper DAO layer, and that we're currently only using the transformation system for a single SigV4-related transformer, it might make sense to proceed with the current approach and revisit the refactor later. WDYT? -- 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
