stevenzwu commented on code in PR #12584:
URL: https://github.com/apache/iceberg/pull/12584#discussion_r3301248166


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -4278,6 +4427,340 @@ components:
         metadata:
           $ref: '#/components/schemas/TableMetadata'
 
+    QueryEventsResponse:
+      type: object
+      required:
+        - continuation-token
+        - highest-processed-timestamp-ms
+        - events
+      properties:
+        continuation-token:
+          type: string
+          description: >
+            An opaque continuation token to fetch the next page of events.
+            This token encodes the server's cursor position and filter state.
+            Clients should treat this as an opaque value and pass it 
unmodified in subsequent requests.
+        highest-processed-timestamp-ms:
+          description: >
+            The highest event timestamp processed when generating this 
response.
+            This may not necessarily appear in the returned changes if it was 
filtered out.
+          type: integer
+          format: int64
+        events:
+          type: array
+          items:
+            $ref: "#/components/schemas/Event"
+
+    Event:
+      type: object
+      required:
+        - event-id
+        - request-id
+        - request-event-count
+        - timestamp-ms
+        - operation
+      properties:
+        event-id:
+          type: string
+          description: Unique ID of this event. Clients should perform 
deduplication based on this ID.
+        request-id:
+          description: >
+            Opaque ID of the request this change belongs to.
+            This ID can be used to identify events that were part of the same 
request.
+            Servers generate this ID randomly.
+          type: string
+        request-event-count:
+          type: integer
+          description: >
+            Total number of events in this batch or request.
+            Some endpoints, such as "updateTable" and "commitTransaction", can 
perform multiple updates in a single atomic request.
+            Each update is modeled as a separate event. All events generated 
by the same request share the same `request-id`.
+            The `request-event-count` field indicates the total number of 
events generated by that request.
+        timestamp-ms:
+          type: integer
+          format: int64
+          description: >
+            Timestamp when this event occurred (epoch milliseconds).
+            Timestamps are not guaranteed to be unique. Typically all events in
+            a transaction will have the same timestamp.
+        actor:
+          type: object
+          description: >
+            The actor who performed the operation, such as a user or service 
account.
+            The content of this field is implementation specific.
+          additionalProperties: true
+        operation:
+          type: object
+          description: >
+            The operation that was performed, such as creating or updating a 
table.
+            Clients should discard events with unknown operation types.
+          discriminator:
+            propertyName: operation-type
+            mapping:
+              create-table: "#/components/schemas/CreateTableOperation"
+              register-table: "#/components/schemas/RegisterTableOperation"
+              drop-table: "#/components/schemas/DropTableOperation"
+              update-table: "#/components/schemas/UpdateTableOperation"
+              rename-table: "#/components/schemas/RenameTableOperation"
+              create-view: "#/components/schemas/CreateViewOperation"
+              drop-view: "#/components/schemas/DropViewOperation"
+              update-view: "#/components/schemas/UpdateViewOperation"
+              rename-view: "#/components/schemas/RenameViewOperation"
+              create-namespace: "#/components/schemas/CreateNamespaceOperation"
+              update-namespace-properties: 
"#/components/schemas/UpdateNamespacePropertiesOperation"
+              drop-namespace: "#/components/schemas/DropNamespaceOperation"
+              custom: "#/components/schemas/CustomOperation"
+          oneOf:
+            - $ref: "#/components/schemas/CreateTableOperation"
+            - $ref: "#/components/schemas/RegisterTableOperation"
+            - $ref: "#/components/schemas/DropTableOperation"
+            - $ref: "#/components/schemas/UpdateTableOperation"
+            - $ref: "#/components/schemas/RenameTableOperation"
+            - $ref: "#/components/schemas/CreateViewOperation"
+            - $ref: "#/components/schemas/DropViewOperation"
+            - $ref: "#/components/schemas/UpdateViewOperation"
+            - $ref: "#/components/schemas/RenameViewOperation"
+            - $ref: "#/components/schemas/CreateNamespaceOperation"
+            - $ref: "#/components/schemas/UpdateNamespacePropertiesOperation"
+            - $ref: "#/components/schemas/DropNamespaceOperation"
+            - $ref: "#/components/schemas/CustomOperation"
+
+    CreateTableOperation:
+      description: >
+        Operation to create a new table in the catalog.
+        Events for this Operation must be issued when the create is finalized 
and committed, not when the create is staged.
+      required:
+        - operation-type
+        - identifier
+        - table-uuid
+        - updates
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "create-table"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        table-uuid:
+          type: string
+          format: uuid
+        updates:
+          type: array
+          items:
+            $ref: "#/components/schemas/TableUpdate"
+
+    RegisterTableOperation:
+      required:
+        - operation-type
+        - identifier
+        - table-uuid
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "register-table"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        table-uuid:
+          type: string
+          format: uuid
+        updates:
+          type: array
+          items:
+            $ref: "#/components/schemas/TableUpdate"
+
+    DropTableOperation:
+      required:
+        - operation-type
+        - identifier
+        - table-uuid
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "drop-table"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        table-uuid:
+          type: string
+          format: uuid
+        purge:
+          type: boolean
+          description: Whether purge flag was set
+
+    UpdateTableOperation:
+      required:
+        - operation-type
+        - identifier
+        - table-uuid
+        - updates
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "update-table"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        table-uuid:
+          type: string
+          format: uuid
+        updates:
+          type: array
+          items:
+            $ref: "#/components/schemas/TableUpdate"
+        requirements:
+          type: array
+          items:
+            $ref: "#/components/schemas/TableRequirement"
+
+    RenameTableOperation:
+      allOf:
+        - $ref: "#/components/schemas/RenameTableRequest"
+      required:
+        - operation-type
+        - table-uuid
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "rename-table"
+        table-uuid:
+          type: string
+          format: uuid
+
+    RenameViewOperation:
+      allOf:
+        - $ref: "#/components/schemas/RenameTableRequest"
+      required:
+        - operation-type
+        - view-uuid
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "rename-view"
+        view-uuid:
+          type: string
+          format: uuid
+
+    CreateViewOperation:
+      required:
+        - operation-type
+        - identifier
+        - view-uuid
+        - updates
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "create-view"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        view-uuid:
+          type: string
+          format: uuid
+        updates:
+          type: array
+          items:
+            $ref: "#/components/schemas/ViewUpdate"
+
+    DropViewOperation:
+      required:
+        - operation-type
+        - identifier
+        - view-uuid
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "drop-view"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        view-uuid:
+          type: string
+          format: uuid
+
+    UpdateViewOperation:
+      required:
+        - operation-type
+        - identifier
+        - view-uuid
+        - updates
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "update-view"
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+        view-uuid:
+          type: string
+          format: uuid
+        updates:
+          type: array
+          items:
+            $ref: "#/components/schemas/ViewUpdate"
+        requirements:
+          type: array
+          items:
+            $ref: "#/components/schemas/ViewRequirement"
+
+    CreateNamespaceOperation:
+      allOf:
+        - $ref: "#/components/schemas/CreateNamespaceResponse"
+      required:
+        - operation-type
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "create-namespace"
+
+    UpdateNamespacePropertiesOperation:
+      allOf:
+        - $ref: "#/components/schemas/UpdateNamespacePropertiesResponse"
+      required:
+        - operation-type
+        - namespace
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "update-namespace-properties"
+        namespace:
+          $ref: "#/components/schemas/Namespace"
+
+    DropNamespaceOperation:
+      required:
+        - operation-type
+        - namespace
+      properties:
+        operation-type:
+          $ref: "#/components/schemas/OperationType"
+          const: "drop-namespace"
+        namespace:
+          $ref: "#/components/schemas/Namespace"
+
+    CustomOperation:
+      type: object
+      description: >
+        Extension point for catalog-specific operations not defined in the 
standard.
+      required:
+        - operation-type
+        - custom-type
+      properties:
+        operation-type:
+          type: string
+          const: "custom"
+        custom-type:
+          $ref: '#/components/schemas/CustomOperationType'
+        # Common optional properties
+        identifier:
+          $ref: "#/components/schemas/TableIdentifier"
+          description: Table or view identifier this operation applies to, if 
applicable
+        namespace:
+          $ref: "#/components/schemas/Namespace"
+          description: Namespace this operation applies to, if applicable
+        table-uuid:

Review Comment:
   are we limiting the custom operation to table and view only? what about 
other object types like functions?



##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -4278,6 +4427,340 @@ components:
         metadata:
           $ref: '#/components/schemas/TableMetadata'
 
+    QueryEventsResponse:
+      type: object
+      required:
+        - continuation-token
+        - highest-processed-timestamp-ms
+        - events
+      properties:
+        continuation-token:
+          type: string
+          description: >
+            An opaque continuation token to fetch the next page of events.
+            This token encodes the server's cursor position and filter state.
+            Clients should treat this as an opaque value and pass it 
unmodified in subsequent requests.
+        highest-processed-timestamp-ms:
+          description: >
+            The highest event timestamp processed when generating this 
response.
+            This may not necessarily appear in the returned changes if it was 
filtered out.
+          type: integer
+          format: int64
+        events:
+          type: array
+          items:
+            $ref: "#/components/schemas/Event"
+
+    Event:
+      type: object
+      required:
+        - event-id
+        - request-id
+        - request-event-count
+        - timestamp-ms
+        - operation
+      properties:
+        event-id:
+          type: string
+          description: Unique ID of this event. Clients should perform 
deduplication based on this ID.
+        request-id:
+          description: >
+            Opaque ID of the request this change belongs to.
+            This ID can be used to identify events that were part of the same 
request.
+            Servers generate this ID randomly.
+          type: string
+        request-event-count:
+          type: integer
+          description: >
+            Total number of events in this batch or request.
+            Some endpoints, such as "updateTable" and "commitTransaction", can 
perform multiple updates in a single atomic request.
+            Each update is modeled as a separate event. All events generated 
by the same request share the same `request-id`.
+            The `request-event-count` field indicates the total number of 
events generated by that request.
+        timestamp-ms:
+          type: integer
+          format: int64
+          description: >
+            Timestamp when this event occurred (epoch milliseconds).
+            Timestamps are not guaranteed to be unique. Typically all events in
+            a transaction will have the same timestamp.
+        actor:
+          type: object
+          description: >
+            The actor who performed the operation, such as a user or service 
account.
+            The content of this field is implementation specific.
+          additionalProperties: true
+        operation:
+          type: object
+          description: >
+            The operation that was performed, such as creating or updating a 
table.
+            Clients should discard events with unknown operation types.

Review Comment:
   The description says "Clients should discard events with unknown operation 
types," but the schema below is closed: `oneOf` enumerates 13 specific schemas 
and `discriminator.mapping` lists the same 13 values, so a strict OpenAPI 
validator (and the codegens that derive from it) will reject any 
`operation-type` outside the mapping rather than discarding it. Concretely, 
when a future spec adds something like `restore-table`, today's generated 
clients won't be able to deserialize that event at all — defeating the "ignore 
unknown" intent stated on `OperationType` and in this description.
   
   A few options:
   - Drop `oneOf` and rely on `discriminator` alone as a dispatch hint. 
Validators that don't enforce closed unions fall through to the base object; 
unknown operations remain decodable as raw JSON, matching the prose contract.
   - Keep `oneOf` but add an explicit "unknown operation" branch (open object, 
no `operation-type` constraint) as a catch-all so codegens emit it as a 
fallback variant.
   - If the spec actually requires clients to fail on unknown types, tighten 
the prose: replace "Clients should discard" with "Clients must reject events 
with operation-type values not defined in this version of the spec," and drop 
the matching sentence on `OperationType`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to