Spec: Clarify field id handling in AddSchemaUpdate and CreateTableRequest#17137
Spec: Clarify field id handling in AddSchemaUpdate and CreateTableRequest#17137sungwy wants to merge 2 commits into
id handling in AddSchemaUpdate and CreateTableRequest#17137Conversation
There was a problem hiding this comment.
Pull request overview
This PR clarifies REST catalog spec semantics around how servers should treat schema field IDs (StructField.id) for schema changes vs table creation, to prevent breaking ID-based references in committed/co-committed artifacts.
Changes:
- Document that REST servers SHOULD preserve client-provided field IDs for
AddSchemaUpdateto avoid invalidating ID-based references. - Document that REST servers MAY (re)assign field IDs for
CreateTableRequestbecause the table has no committed data yet. - Mirror the OpenAPI YAML text in the generated Pydantic model docstrings.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| open-api/rest-catalog-open-api.yaml | Adds/updates schema descriptions for AddSchemaUpdate and CreateTableRequest to clarify field ID handling. |
| open-api/rest-catalog-open-api.py | Adds corresponding docstrings to the generated Pydantic models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| Table creation request. The field IDs in `schema` are assigned by the client, but because a newly created table has no committed data, a server MAY (re)assign them when creating the table. | ||
| """ |
| description: | ||
| Table creation request. The field IDs in `schema` are assigned by the client, but because a newly | ||
| created table has no committed data, a server MAY (re)assign them when creating the table. |
| Adds a schema to the table. The field IDs in `schema` are assigned by the client. The client | ||
| assigns them so that the schema stays consistent with the data files, partition specs, and | ||
| sort orders it writes or co-commits, which reference fields by their field ID | ||
| (`StructField.id`). Servers SHOULD preserve the field IDs submitted by the client on |
There was a problem hiding this comment.
I made this SHOULD for now, but my preference is to make this a MUST.
The invariant we need to preserve in the catalog server is that the field IDs that are committed are consistent with the constructs (partition spec, sort order, data files) that are committed together with it.
The only way to ensure that invariant are:
preserve the field IDs provided by the client (assuming that the client guarantees consistency within its commit)
reassign field IDs as it wants, and check and rewrite the data files with the new field IDs if needed
I don't think (2) is practical because the data isn't sent over in the payload of the AddSchemaUpdate in CommitTableRequest, only the pointers to it are (through chain of pointers). A server that holds storage credentials could step outside the commit path to scan and rewrite every affected file, but that's an O(data) operation on a metadata commit.
Making (1) a part of the protocol seems practical, reflects the current state of catalog server implementations, and makes the behavior verifiable via conformance tests (RCK can assert it on round-trip)
There was a problem hiding this comment.
I'd argue that this should be a MUST.
Let's say that I'm trying to add a new column and then immediately rename it in the same update (yes, this is a silly example)
- AddSchemaUpdate adds the new column. User specifies this as ID 10, but catalog ignores it and uses ID 20 instead
- In that same update, I attempt to add a new schema with AddSchemaUpdate. In that schema, I attempt to rename the column on ID 10. But, the catalog has already renamed it to ID 20.
The REST catalog spec does not explicitly state how a REST Catalog Server should handle field IDs on schema updates.
This PR clarifies the docs to state:
AddSchemaUpdate, as doing so would make the schema inconsistent with the field IDs already referenced by committed or co-committed data files.CreateTableRequest, since a new table has no committed data referencing them yet.This matches the behavior implemented in Iceberg core, which the reference REST server (RCK / iceberg-rest-fixture) is built on, and which open source implementations such as Apache Polaris reuse.