Generating code based on the spec is completely different than generating code based on the annotations introduced by Immutables.
More importantly, we shouldn’t allow a discussion about pushing the project forward to devolve into a debate around some third-party library that’s not necessary to the change. On Mon, Feb 17, 2025 at 9:42 AM Robert Stupp <sn...@snazy.de> wrote: > Everything you have to code manually requires explicit tests. > > Immutables is a very mature library that has exceptionally good test > coverage that prevents a ton of accidental coding mistakes. We've been > using it for many many years in multiple projects and literally never > produced wrong code. > > We already have code generators in the code base for openapi. I'd okay > to remove that, because the code generated by that generator is IMHO not > great. > > Overall we should focus on the "real stuff" and not bother about > manually writing builders and really immutable types. > > On 17.02.25 18:10, Michael Collado wrote: > > I prefer native Java constructs over third party libraries and compiler > > plugins, whenever possible. I’m a fan of Java records. > > > > Mike > > > > On Mon, Feb 17, 2025 at 6:55 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > wrote: > > > >> Hi Robert, > >> > >> Yes, my intention about Java records (e.g. > >> https://openjdk.org/jeps/395) is to leverage: > >> - Devise an object-oriented construct that expresses a simple > >> aggregation of values. > >> - Focus on modeling immutable data rather than extensible behavior. > >> - Automatically implement data-driven methods such as equals and > accessors. > >> - Preserve long-standing Java principles such as nominal typing and > >> migration compatibility. > >> - Provide inner builder, optionally with technical validation > >> (Objects.NotNull, etc), for instance: > >> > >> public record CatalogDAO(String id, String name, ...) { > >> > >> public static final class Builder { > >> String id; > >> String name; > >> public Builder() {} > >> public Builder id(String id) { > >> this.id = id; > >> return this; > >> } > >> public Builder name(String name) { > >> this.name = name; > >> return this; > >> } > >> public CatalogDAO build() { > >> return new CatalogDAO(id, name); > >> } > >> } > >> > >> } > >> NB: that's just a "raw" example :) > >> > >> That could help us for the "DAO" layer, with clean isolation and data > >> "transport". The "conversion" from Polaris Entity to DAO could be > >> performed by the intermediate logic layer (e.g. > >> PolarisMetaStoreManager), the pure persistence layer can deal with DAO > >> only (e.g. PolarisStore). > >> > >> Good point about immutable collections. In some projects, I "force" > >> the copy of a collection to ensure immutability, something like: > >> > >> record NamespaceDAO(Set<String> children) { > >> public NamespaceDAO { > >> children = Set.copyOf(children); > >> } > >> } > >> > >> OK, that's not super elegant :) but it does the trick ;) > >> > >> Regards > >> JB > >> > >> On Mon, Feb 17, 2025 at 5:28 PM Robert Stupp <sn...@snazy.de> wrote: > >>> Agree with JB, except I think "immutables" serves the need much better. > >>> Java records are okay, but do not ensure that all fields are immutable > - > >>> especially collections. The other pro of immutables is that there are > >>> proper, descriptive builders - hence no need to constructors with a > >>> gazillion parameters. > >>> > >>> On 17.02.25 11:42, Jean-Baptiste Onofré wrote: > >>>> Hi Yufei > >>>> > >>>> I left comments in the PR. > >>>> > >>>> Thanks for that. That's an interesting approach but slightly different > >>>> to what I had in mind. > >>>> > >>>> My proposal was: > >>>> > >>>> 1. The DAOs are Java records clearly descriptive, without "storage > >> operations". > >>>> For instance, we can imagine: > >>>> > >>>> public record CatalogDao(String id, String name, String location, ...) > >> { > >>>> ... > >>>> } > >>>> > >>>> public record NamespaceDao(String id, String name, String parent, ...) > >> {} > >>>> public record TableDao(String id, String name, ...) {} > >>>> > >>>> etc > >>>> > >>>> The advantages are: > >>>> - very simple and descriptive > >>>> - common to any backend implementation > >>>> > >>>> 2. The PolarisStore defines the DAO backend operations and mapping to > >>>> "internal" Polaris entities. Each persistence adapter implements the > >>>> PolarisStore. > >>>> For the persistence adapter implementer, he just has to implement the > >>>> DAO persistence (no need to understand other Polaris parts). > >>>> Each persistence adapter is in its own module (for isolation and > >> dependencies). > >>>> During the Polaris Persistence Meeting last week, we already got > >>>> consensus on the approach proposed by Dennis and I. I propose to do a > >>>> new sync/review with Dennis. > >>>> > >>>> Regards > >>>> JB > >>>> > >>>> On Mon, Feb 17, 2025 at 9:40 AM Yufei Gu <flyrain...@gmail.com> > wrote: > >>>>> Hi Folks: > >>>>> > >>>>> Here is a POC for persistence layer refactor. Please check it out and > >> let > >>>>> me know what you think. Please note this is a POC, we still need a > >> lot of > >>>>> effort to complete the refactor. > >>>>> > >>>>> PR: https://github.com/apache/polaris/pull/1011. > >>>>> Design doc: > >>>>> > >> > https://docs.google.com/document/d/1Vuhw5b9-6KAol2vU3HUs9FJwcgWtiVVXMYhLtGmz53s/edit?tab=t.0 > >>>>> Experiment: > >>>>> > >>>>> - Added a DAO layer for the business entity namespace(except the > >> read). > >>>>> - Integrated with existing DAO components > >> (PolarisMetaStoreManager and > >>>>> PolarisMetaStoreSession). > >>>>> - All tests passed successfully, including a manual local run > >> with Spark > >>>>> sql. > >>>>> > >>>>> Benefits: > >>>>> > >>>>> - Compatible with the existing backend(FDB), as we hide it > behind > >> the > >>>>> new DAO. > >>>>> - Adding new backends(Postgres/MongoDB) is much easier now, esp > >> for > >>>>> Postgres, we could be able to use a similar model as Iceberg > Jdbc > >> catalog. > >>>>> - Allows gradual refactoring to remove old DAO dependencies > >>>>> (PolarisMetaStoreManager and PolarisMetaStoreSession). > >>>>> - Enables parallel development of new backend implementations. > >>>>> > >>>>> Next Steps: > >>>>> > >>>>> - Define business entities one by one to decouple them from FDB. > >>>>> - Create DAO interfaces for each entity to standardize > operations > >> (e.g., > >>>>> CRUD, ID generation). > >>>>> - Expand DAO implementations to support additional backends over > >> time. > >>>>> > >>>>> > >>>>> > >>>>> Yufei > >>> -- > >>> Robert Stupp > >>> @snazy > >>> > -- > Robert Stupp > @snazy > >
