sungwy commented on code in PR #17137:
URL: https://github.com/apache/iceberg/pull/17137#discussion_r3545980661
##########
open-api/rest-catalog-open-api.py:
##########
@@ -1516,6 +1516,10 @@ class ViewMetadata(BaseModel):
class AddSchemaUpdate(BaseUpdate):
+ """
+ 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 add-schema, as reassigning them would
require re-deriving and rewriting every such reference, including in
already-written data files.
Review 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:
1. preserve the field IDs provided by the client (assuming that the client
guarantees consistency within its commit)
2. 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)
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3120,6 +3120,13 @@ components:
AddSchemaUpdate:
allOf:
- $ref: '#/components/schemas/BaseUpdate'
+ description:
+ 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
Review 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)
--
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]
