Flatten events hierarchy #3293

olsoloviov · 2025-12-18T09:41:44Z

Flatten Polaris events structure to have a single PolarisEvent implementation with enum type and custom attributes.
This is an implementation of @adutra proposal: https://lists.apache.org/thread/xonxwf9b38t9cxo841r0hn1b34plf7og

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

adutra

Very nice work @olsoloviov, that is very much what I had in mind. Left a few minor comments Thanks!

runtime/service/src/main/java/org/apache/polaris/service/events/EventAttributes.java

adutra · 2025-12-18T12:56:13Z

runtime/service/src/main/java/org/apache/polaris/service/events/EventAttributes.java

+  @SuppressWarnings("unchecked")
+  public static final AttributeKey<Map<String, String>> NAMESPACE_PROPERTIES =
+      (AttributeKey<Map<String, String>>)
+          (AttributeKey<?>) AttributeKey.of("namespace_properties", Map.class);


I'm wondering: in order to increase portability, shouldn't we restrict the types of attributes that we can put in this map? An attribute of type Map<String, String> is still OK, I guess, but what if the attribute doesn't have a clear serialized format, e.g. Optional or Function? Restricting to types that can be safely serialized to Json (and maybe gRPC) would imho make things easier for listener implementors.

Shouldn't it be handled during serialization phase? We will have pruning anyway. Also, not sure if it is a desirable scenario, but some non-serializable helper context could be passed with an event.
If you strongly prefer to restrict the types, we could whitelist the types during attributes creation, wdyt @adutra?

Admittedly I haven't thought through all the ramifications, but initially I agree with @adutra on this one. Maybe we just keep the bar that whatever the object is, it must be implementing Serializable?

Personally, I'd rather have this enforced at the time of the event generation than having to deal with it later in the Event lifecycle - but not a strong opinion.

I have added a white list for allowed attributes types

While widely adopted in the Hadoop ecosystem, Serializable is generally viewed as a legacy feature in Java. Leveraging this marker interface here imho wouldn't achieve our core objective, which is to ensure the attributes map can be easily serialized using common wire formats, particularly JSON. For example, many objects that we want to allow aren't Serializable, e.g. the request and response types.

All of this to say: if my suggestion proves too difficult to implement, I am fine if we don't enforce anything.

.../main/java/org/apache/polaris/service/events/jsonEventListener/PropertyMapEventListener.java

adutra · 2025-12-18T13:05:18Z

...c/main/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListener.java

  }

-  protected abstract void processEvent(String realmId, PolarisEvent event);
+  private <T> T getRequiredAttribute(PolarisEvent event, AttributeKey<T> key) {


This could be declared in PolarisEvent, I think it's going to be useful for many listeners, not just this one.

Moved to PolarisEvent, also switched to throw instead of logging

adutra · 2025-12-18T13:08:24Z

...c/main/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListener.java

+    }
+  }
+
+  private void handleAfterCreateTable(PolarisEvent event) {


Again, with flattened events hierarchy there is not a huge difference between these two handleXYZ methods. It would certainly be possible to create a unified handleEvent method that is valid for all events, thus saving us the hassle of writing 150+ methods (just a thought for later, not for this PR though).

runtime/service/src/main/java/org/apache/polaris/service/events/AttributeKey.java

adnanhemani

A very nice job done on this one, @olsoloviov - thank you so much! I left a handful of nit comments and a few minor ones as well, but I think this is very close to merging!

adnanhemani · 2025-12-24T23:27:06Z

runtime/service/src/main/java/org/apache/polaris/service/events/EventAttributes.java

+  @SuppressWarnings("unchecked")
+  public static final AttributeKey<Map<String, String>> NAMESPACE_PROPERTIES =
+      (AttributeKey<Map<String, String>>)
+          (AttributeKey<?>) AttributeKey.of("namespace_properties", Map.class);


Admittedly I haven't thought through all the ramifications, but initially I agree with @adutra on this one. Maybe we just keep the bar that whatever the object is, it must be implementing Serializable?

Personally, I'd rather have this enforced at the time of the event generation than having to deal with it later in the Event lifecycle - but not a strong opinion.

adnanhemani · 2025-12-24T23:32:49Z

...ice/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogTest.java

-    Assertions.assertThat(beforeRefreshEvent.tableIdentifier()).isEqualTo(TestData.TABLE);
+    PolarisEvent beforeRefreshEvent =
+        testPolarisEventListener.getLatest(PolarisEventType.BEFORE_REFRESH_TABLE);
+    Assertions.assertThat(beforeRefreshEvent.attribute(EventAttributes.TABLE_IDENTIFIER))


nit: Let's test these using requiredAttribute rather than attribute?

Commenting in case you forgot to make this change :)

adnanhemani · 2025-12-24T23:35:56Z

runtime/service/src/test/java/org/apache/polaris/service/events/AttributeKeyTest.java

+  }
+
+  @Test
+  void testTypeCastValidValue() {


Not sure I see value in this test case. Maybe do something like making an AttributeKey of a Long value, then give it an int and see if it casted that properly?

IOW, we shouldn't really be testing cast if we didn't write it. We should be testing the code that we wrote.

Thanks, @adnanhemani, for your comments, I agree that it does not make any sense to test Class.cast()

adnanhemani · 2025-12-24T23:39:53Z

runtime/service/src/test/java/org/apache/polaris/service/events/AttributeKeyTest.java

+  }
+
+  @Test
+  void testTypeCastInvalidType() {


Similar to above, I would move this test into the test for PolarisEvent and test that sending an int to an AttributeKey would throw when calling key.attribute(..., 123).

adnanhemani · 2025-12-24T23:41:34Z

runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventTest.java

+  void testBuilderCreatesEvent() {
+    PolarisEvent event =
+        PolarisEvent.builder(PolarisEventType.BEFORE_CREATE_TABLE, TEST_METADATA)
+            .attribute(EventAttributes.CATALOG_NAME, "my-catalog")


nit: extract the magic variables for maintainability

adnanhemani · 2025-12-24T23:43:28Z

runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventTest.java

+
+    assertThatThrownBy(() -> event.requiredAttribute(EventAttributes.CATALOG_NAME))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessageContaining("catalog_name")


nit: Can we test for a more descriptive message? Not looking for exact string matching but something similar to "Required attribute" and/or "not found in event".

adutra · 2026-01-06T15:15:21Z

runtime/service/src/main/java/org/apache/polaris/service/events/AllowedAttributeTypes.java

+          Boolean.class,
+          Integer.class,
+          Long.class,
+          Double.class,


Missing Float? Also: we could probably authorize any type that extends Number, including BigInteger and BigDecimal.

adutra · 2026-01-06T15:16:22Z

runtime/service/src/main/java/org/apache/polaris/service/events/AllowedAttributeTypes.java

+          Long.class,
+          Double.class,
+          // Collections
+          List.class,


This is probably too loose. We are not checking the collections element types here, so one could include a list or a set of any element type, effectively beating the purpose of this class in the first place. I would suggest to also check the element type and the map's key and value types: these should conform to the list specified here as well.

adutra · 2026-01-06T15:19:34Z

runtime/service/src/main/java/org/apache/polaris/service/events/AllowedAttributeTypes.java

+          TableMetadata.class,
+          ViewMetadata.class,
+          // Iceberg REST request types
+          CreateNamespaceRequest.class,


Maybe you could check if the type implements RESTMessage?

Here is a suggestion that leverages TypeToken in order to check element types of List, Set and Map:

static final Set<Class<?>> ALLOWED_TYPES = Set.of( String.class, Boolean.class, Number.class, RESTMessage.class, Namespace.class, TableIdentifier.class, TableMetadata.class, ViewMetadata.class, Catalog.class, Principal.class, PrincipalRole.class, PrincipalWithCredentials.class, CatalogRole.class, GrantResource.class, PolarisPrivilege.class, GenericTable.class); static boolean isAllowed(TypeToken<?> type) { Class<?> rawType = type.getRawType(); if (rawType.equals(List.class)) { TypeToken<?> elementType = type.resolveType(List.class.getTypeParameters()[0]); return isSubtypeOfAllowedType(elementType.getRawType()); } else if (rawType.equals(Set.class)) { TypeToken<?> elementType = type.resolveType(Set.class.getTypeParameters()[0]); return isSubtypeOfAllowedType(elementType.getRawType()); } else if (rawType.equals(Map.class)) { TypeToken<?> keyType = type.resolveType(Map.class.getTypeParameters()[0]); TypeToken<?> valueType = type.resolveType(Map.class.getTypeParameters()[1]); return isSubtypeOfAllowedType(keyType.getRawType()) && isSubtypeOfAllowedType(valueType.getRawType()); } else { return isSubtypeOfAllowedType(rawType); } } private static boolean isSubtypeOfAllowedType(Class<?> rawType) { return ALLOWED_TYPES.stream().anyMatch(t -> t.isAssignableFrom(rawType)); }

I've seen the updated version, that's much better, thanks! But I'm now concerned about the performance aspect. All this computation is done for every event attribute. I am worried that this is a bit too much. @adnanhemani and @olsoloviov, wdyt? Am I overthinking this?

adutra · 2026-01-06T15:33:12Z

runtime/service/src/main/java/org/apache/polaris/service/events/AttributeKey.java

+ */
+public final class AttributeKey<T> {
+  private final String name;
+  private final Class<T> type;


I suggest that we use Guava's com.google.common.reflect.TypeToken here, as it handles generic types as well.

adutra · 2026-01-06T15:48:33Z

runtime/service/src/main/java/org/apache/polaris/service/events/AttributeKey.java

+ *
+ * @param <T> the type of the attribute value
+ */
+public final class AttributeKey<T> {


This class could be a record. Then the constructor could be public and we wouldn't need the static of method.

The following should be enough (also introducing support for TypeToken):

public record AttributeKey<T>(@JsonValue String name, TypeToken<T> type) { public AttributeKey(String name, Class<T> type) { this(name, TypeToken.of(type)); } public AttributeKey(String name, TypeToken<T> type) { this.name = Objects.requireNonNull(name, "name"); this.type = Objects.requireNonNull(type, "type"); if (!AllowedAttributeTypes.isAllowed(type)) { throw new IllegalArgumentException("Type " + type + " is not allowed for event attributes"); } } }

adutra · 2026-01-06T15:53:03Z

runtime/service/src/main/java/org/apache/polaris/service/events/EventAttributes.java

+      AttributeKey.of("register_table_request", RegisterTableRequest.class);
+  public static final AttributeKey<RenameTableRequest> RENAME_TABLE_REQUEST =
+      AttributeKey.of("rename_table_request", RenameTableRequest.class);
+  public static final AttributeKey<TableMetadata> TABLE_METADATA_BEFORE =


My IDE is flagging some attributes as unused, which is intriguing. This is one of them.

Thanks for catching, these are not needed after ...CommitTable events were removed

adutra · 2026-01-06T16:23:07Z

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEvent.java

+ * Represents an event emitted by Polaris. Events have a type, metadata, and a map of typed
+ * attributes. Use {@link #builder(PolarisEventType, PolarisEventMetadata)} to create instances.
+ */
+public record PolarisEvent(


FYI we can leverage the Java Immutables library to create the builder class for us. Here is how to do it:

@Value.Style(depluralize = true) public record PolarisEvent( PolarisEventType type, PolarisEventMetadata metadata, Map<AttributeKey<?>, Object> attributes) { @Builder.Constructor public PolarisEvent { ... } }

Then we can create an event like this:

PolarisEvent event = new PolarisEventBuilder() .type(PolarisEventType.BEFORE_LIMIT_REQUEST_RATE) .metadata(eventMetadataFactory.create()) .putAttribute(EventAttributes.HTTP_METHOD, ctx.getMethod()) .putAttribute( EventAttributes.REQUEST_URI, ctx.getUriInfo().getAbsolutePath().toString()) .build();

Another option is to transform the record into an interface and annotate with @PolarisImmutable.

Thanks for the notion. I see only one issue with such approach - putAttribute() will not be type-safe, e.g. something like .putAttributes(EventAttributes.REQUEST_URI, 123) will compile

That's right indeed. But now that we have AttributeMap, shouldn't we move those put methods to AttributeMap?

I implemented this idea, let me know what you all think:

adutra@2286bfd68

I think @adutra's idea is likely better. Let's go with it.

adutra · 2026-01-06T16:23:46Z

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEvent.java

+    PolarisEventType type, PolarisEventMetadata metadata, Map<AttributeKey<?>, Object> attributes) {
+
+  public PolarisEvent {
+    attributes = Collections.unmodifiableMap(new HashMap<>(attributes));


Suggested change

attributes = Collections.unmodifiableMap(new HashMap<>(attributes));

attributes = Map.copyOf(attributes);

adutra · 2026-01-06T16:30:08Z

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEvent.java

+ * attributes. Use {@link #builder(PolarisEventType, PolarisEventMetadata)} to create instances.
+ */
+public record PolarisEvent(
+    PolarisEventType type, PolarisEventMetadata metadata, Map<AttributeKey<?>, Object> attributes) {


Nit: exposing a Map<AttributeKey<?>, Object> parameter is a bit low-level. It would be better to introduce an AttributeMap class with type-safe operations for get and put. That's what Netty does, cf. io.netty.util.AttributeMap.

Very simple impl suggestion:

public final class AttributeMap { private final Map<AttributeKey<?>, Object> attributes = new HashMap<>(); @SuppressWarnings("unchecked") public <T> Optional<T> get(AttributeKey<T> key) { return Optional.ofNullable((T) attributes.get(key)); } public <T> T getRequired(AttributeKey<T> key) { return get(key) .orElseThrow(() -> new IllegalStateException("Required attribute " + key + " not found")); } }

I'm 50/50 on this being a requirement, but willing to go with @adutra's suggestion here :)

Thanks, that makes sense. It is not compatible with using Immutables on PolarisEvent though - putAttribute() will not be generated, so we will have to stick with manual builder.

olsoloviov · 2026-01-08T12:13:17Z

Thanks for your comprehensive comments, @adutra! I have implemented most of your suggestions in the new commit. Immutables-generated builder does not work nice with AttributeMap though, so I stuck with manual builder.

adnanhemani · 2026-01-08T21:11:02Z

...c/main/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListener.java

+    Namespace namespace = event.requiredAttribute(EventAttributes.NAMESPACE);
+    String tableName = event.requiredAttribute(EventAttributes.TABLE_NAME);
+
+    org.apache.polaris.core.entity.PolarisEvent polarisEvent =


nit: not sure why we had to write the full class hierarchy here. Was there a change?

Well, yeah. This is another PolarisEvent from polaris-core. It was imported before, but now we had to import our org.apache.polaris.service.events.PolarisEvent, so now we have to fully qualify that one.

adnanhemani · 2026-01-08T21:23:22Z

runtime/service/src/main/java/org/apache/polaris/service/events/AllowedAttributeTypes.java

+  }
+
+  private static boolean isSubtypeOfAllowedType(Class<?> rawType) {
+    return ALLOWED_TYPES.stream().anyMatch(t -> t.isAssignableFrom(rawType));


Not sure if this is too nit-picky - but maybe something like this to ensure we don't waste a lot of cycles going through this check for the same handful of classes?

private static final ClassValue<Boolean> IS_ALLOWED = new ClassValue<>() { @Override protected Boolean computeValue(Class<?> type) { for (Class<?> allowed : ALLOWED_TYPES) { if (allowed.isAssignableFrom(type)) { return true; } } return false; } }; private static boolean isSubtypeOfAllowedType(Class<?> rawType) { return IS_ALLOWED.get(rawType); }

adnanhemani · 2026-01-08T21:57:30Z

...ice/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogTest.java

-    Assertions.assertThat(beforeRefreshEvent.tableIdentifier()).isEqualTo(TestData.TABLE);
+    PolarisEvent beforeRefreshEvent =
+        testPolarisEventListener.getLatest(PolarisEventType.BEFORE_REFRESH_TABLE);
+    Assertions.assertThat(beforeRefreshEvent.attribute(EventAttributes.TABLE_IDENTIFIER))


Commenting in case you forgot to make this change :)

adnanhemani · 2026-01-08T22:01:38Z

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEvent.java

+ * Represents an event emitted by Polaris. Events have a type, metadata, and a map of typed
+ * attributes. Use {@link #builder(PolarisEventType, PolarisEventMetadata)} to create instances.
+ */
+public record PolarisEvent(


I think @adutra's idea is likely better. Let's go with it.

olsoloviov added 2 commits December 17, 2025 12:56

flatten event hierarchy into single type with custom attributes

38bbaa2

fix event listener tests

8ecd145

github-project-automation bot added this to Basic Kanban Board Dec 18, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Dec 18, 2025

adutra reviewed Dec 18, 2025

View reviewed changes

address some reviewers comments

b3ddb4d

adutra reviewed Dec 18, 2025

View reviewed changes

runtime/service/src/main/java/org/apache/polaris/service/events/AttributeKey.java Outdated Show resolved Hide resolved

dimas-b reviewed Dec 18, 2025

View reviewed changes

runtime/service/src/main/java/org/apache/polaris/service/events/AttributeKey.java Outdated Show resolved Hide resolved

address some more reviewers comments

1cb61f1

adnanhemani reviewed Dec 24, 2025

View reviewed changes

olsoloviov added 2 commits January 5, 2026 14:45

add a whitelist for allowed types of event attributes

1ace715

addressing reviewers comments for unit tests, applying spotless

0772878

adutra reviewed Jan 6, 2026

View reviewed changes

use TypeToken, some refactoring

cc50e4a

adnanhemani reviewed Jan 8, 2026

View reviewed changes

adnanhemani mentioned this pull request Jan 9, 2026

[DO NOT MERGE] POC for Using Intermediate Data as part of Events #3391

Draft

6 tasks

	attributes = Collections.unmodifiableMap(new HashMap<>(attributes));
	attributes = Map.copyOf(attributes);

Flatten events hierarchy #3293

Are you sure you want to change the base?

Flatten events hierarchy #3293

Uh oh!

Conversation

olsoloviov commented Dec 18, 2025

Checklist

Uh oh!

adutra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olsoloviov Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adnanhemani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adutra Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olsoloviov Dec 18, 2025 •

edited

Loading

adutra Jan 8, 2026 •

edited

Loading

adutra Jan 6, 2026 •

edited

Loading

adnanhemani Jan 7, 2026 •

edited

Loading