Hi All, >From my POV we should flag events as "beta" in 1.2.0 release notes and drop/re-create events tables on upgrade.
However, I do not think we have technical means to force JDBC schema changes on upgrade. At best we can disable event persistence (with ERROR log messages) if the schema is old and wait for the user to run the admin tool to apply schema changes. Cheers, Dmitri. On Fri, Oct 24, 2025 at 5:26 AM Alexandre Dutra <[email protected]> wrote: > Hi all, > > Thank you for your input here. Mike and Adnan: I'm fine with numerical > ids in the database, let's keep using this pattern. > > I think the only question remaining is how to handle the SQL schema > migration. Currently we have an event_type column of type TEXT NOT > NULL [1]. We would need instead an event_type_code column of type > INTEGER NOT NULL. > > Like Dmitri, I confess I do not recall if the Events API is considered > beta or production-ready at this point. Also, the events table is > probably meant to be purged regularly. Do you think it would be OK to > just drop the existing column and introduce a new one, also > instructing users to drop and re-create the table when upgrading > Polaris? > > Thanks, > Alex > > [1]: > https://github.com/apache/polaris/blob/e02bb7171ba7af1aeba224e4d5fd585ea6c88624/persistence/relational-jdbc/src/main/resources/postgres/schema-v3.sql#L129 > > On Fri, Oct 24, 2025 at 9:48 AM Adnan Hemani > <[email protected]> wrote: > > > > Hi Alex, > > > > I agree with your concerns. Here is what I am recommending: > > * +1 to Michael’s recommendation of keeping numeric IDs for storage > purposes. > > * Waiting on the logical IDs until we have a use case for it. I’m not > sure the configurations use case is fully defined yet (if you think it is, > happy to discuss on a different mailing thread so as to not hijack this > thread). But we should add a new method regardless to the PolarisEvent > interface: `int typeId()` or something similar - and use that forward in > storage. > > > > Best, > > Adnan Hemani > > > > > On Oct 23, 2025, at 9:23 AM, Michael Collado <[email protected]> > wrote: > > > > > > Totally agree. In fact, other enum types (e.g., PolarisPrivilege, > > > PolarisEntityType) use numeric ids in storage to decouple naming in > code > > > from the values we persist in the database. I'd recommend continuing > with > > > that pattern. > > > > > > Mike > > > > > > On Wed, Oct 22, 2025 at 10:38 AM Dmitri Bourlatchkov <[email protected] > <mailto:[email protected]>> > > > wrote: > > > > > >> Hi Alex, > > >> > > >> +1 to all three counts. > > >> > > >> As for the SQL migration script, is event persistence a fully > supported > > >> feature or a "beta"... TBH, I do not recall how it was presented to > users. > > >> WDYT? > > >> > > >> Cheers, > > >> Dmitri. > > >> > > >> On Wed, Oct 22, 2025 at 9:44 AM Alexandre Dutra <[email protected]> > wrote: > > >> > > >>> Hi all, > > >>> > > >>> Currently, Polaris manages over 150 event types, but they lack proper > > >>> identifiers, leading to the problematic use of Java class names to > > >>> represent the event types. > > >>> > > >>> For instance, PolarisPersistenceEventListener relies on Java class > > >>> names to store event types in the database [1]. Thus, renaming or > > >>> moving an event class could inadvertently break this listener, or any > > >>> other listener doing something similar. > > >>> > > >>> Moreover, if we were to introduce a configuration to manage event > > >>> types (which I think we'll have to do eventually), we'd need to use > > >>> Java class names again, out of better options. Consider a > hypothetical > > >>> configuration example that highlights this issue: > > >>> > > >>> polaris.event-listener.enabled-event-types=\ > > >>> > > >>> > > >> > org.apache.polaris.service.events.IcebergRestCatalogEvents.AfterRefreshTableEvent,\ > > >>> > > >>> > > >> > org.apache.polaris.service.events.IcebergRestCatalogEvents.AfterCommitTableEvent > > >>> > > >>> It's obvious that listing event types in such a way is not only > > >>> verbose, but also cryptic for users. It introduces an unnecessary > > >>> tight coupling between user-facing configuration and Polaris > > >>> internals. > > >>> > > >>> To address these concerns, I would like to promote event types to > > >>> first-class citizens in Polaris: > > >>> > > >>> 1. Introduce the concept of an event type with a unique, logical ID > > >>> (e.g., BEFORE_CREATE_CATALOG). This could be enforced through a Java > > >>> enum for uniqueness. > > >>> 2. Add a new method, PolarisEventType type(), to the PolarisEvent > > >>> interface. This is not only informative; it could serve e.g. as a > type > > >>> discriminator when serializing events. > > >>> 3. Replace class names with logical IDs, both in configuration and > > >>> when storing events in the database. > > >>> > > >>> Note: The last item represents a breaking change and may require a > SQL > > >>> migration script. > > >>> > > >>> I'm curious to hear your thoughts. > > >>> > > >>> Thanks, > > >>> Alex > > >>> > > >>> [1]: > > >>> > > >> > https://www.google.com/url?q=https://github.com/apache/polaris/blob/3b4b0169032216d85d48130ad43285f86e886e3c/runtime/service/src/main/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListener.java%23L44&source=gmail-imap&ust=1761841473000000&usg=AOvVaw0v18HDrE4mH9C3INPPhtLp > > >
