OpenAI: IRC spec changes for v4 [WIP][Discuss] - #17543
Draft
dhruvarya-db wants to merge 4 commits into
Draft
Conversation
Opening this to drive discussion on what the REST catalog spec needs for
format version 4. It is a proposal rather than a merge candidate: parts of it
depend on table spec changes that are still in review, and one change is
deliberately breaking. Details and open questions below.
Format version 4 restructures table metadata, and three constraints in the
current spec prevent a catalog from serving a v4 table at all: format-version
is capped at 3, Snapshot requires manifest-list, and CommitTableResponse
requires metadata-location.
Changes:
- TableMetadata: raise the format-version maximum to 4, and document that
location is optional in v4, where it may be supplied by the catalog rather
than carried in metadata.
- Snapshot: add root-manifest and make manifest-list optional, with a oneOf
requiring exactly one of the two. A snapshot with neither is invalid and must
not be read as a snapshot with no files. Which of the two applies is
determined by format-version, which cannot be expressed here because that
field lives in the enclosing TableMetadata, so it is stated normatively.
- LoadTableResult and CommitTableResponse: add table-location, which carries the
table's base location for resolving relative paths. Format version 4 makes
TableMetadata.location optional while allowing relative paths, so a table can
have metadata that is unreadable without a catalog-supplied base.
CommitTableResponse no longer requires metadata-location, and both responses
document that it may be absent when the catalog is the source of truth for
table state and no client-visible metadata pointer exists.
- UnregisterTableResult: add table-location, so the endpoint that hands back a
metadata location cannot return metadata with no base to resolve against.
- Move the table endpoints to /v2. Every path whose response can carry table
metadata moves: tables, tables/{table}, register, and tables/{table}/unregister.
Open questions:
- root-manifest is not in the ratified spec. It comes from the adaptive metadata
tree proposal (apache#16025), which is still open, so the field name and shape may
change. Note also that SnapshotParser currently writes manifest-list for v4
tables, so the oneOf as written would reject snapshots this repository
produces today. Whether to keep the field, defer it, or make it purely
additive is the main thing worth discussing.
- Moving the table endpoints to /v2 rather than adding /v2 alongside /v1 is a
breaking change, and assumes v1 is no longer supported. An additive version
that leaves /v1 in place is the obvious alternative if that assumption does
not hold.
- Whether path versioning is the right mechanism at all, versus a header or a
content-type parameter.
- Relative path support and the typed content stats are already in the spec for
v4; snapshot offloading is not yet, and would interact with the snapshots
query parameter on loadTable if it lands later.
make lint and make generate both pass.
Renames the field from root-manifest to content-root, and follows the name in
the surrounding descriptions so the prose matches ("the snapshot's content
root" rather than "the snapshot's root manifest").
make lint and make generate both pass, and the generated model picks up the
rename in both oneOf branches.
dhruvarya-db
changed the title
dhruvarya-db
changed the title
…ield Reworks the change so v1 endpoints stay in place unchanged, and the format version 4 behavior lives on parallel v2 endpoints, rather than moving the table endpoints to v2 wholesale. Naming follows the convention that the canonical name is the new (v4-capable) variant, edited in place, and the frozen original gets a V1 suffix. A V1 twin is created only where the canonical schema relaxes a required field to optional, so that v1 keeps the original required-ness: - Snapshot relaxes manifest-list -> SnapshotV1 keeps it required. - CommitTableResponse relaxes metadata-location -> CommitTableResponseV1 keeps it required. Because Snapshot is reached only through TableMetadata.snapshots, isolating the v1 chain requires frozen copies of the containers as well, so v1 endpoints route through LoadTableResultV1 / TableMetadataV1 / UnregisterTableResultV1 and the corresponding V1 response wrappers. These containers are not themselves relaxing anything; they exist only to carry the frozen SnapshotV1 and keep the v1 wire contract byte-identical. TableMetadataV1 keeps format-version max 3, so v1 responses cannot represent a v4 table at all. Schemas that only add an optional field (LoadTableResult, UnregisterTableResult gain table-location; TableMetadata raises the format-version max) are shared: adding an optional field does not change the v1 contract, so no twin is needed there beyond the container copies required for Snapshot isolation. Review note: the v1-side additions (the *V1 schemas, response wrappers, and the re-added /v1 paths) are verbatim copies of the current definitions and can be read as "unchanged from before" -- verified that each generated *V1 model is byte-identical to its upstream original, and each /v1 path is byte-identical to upstream modulo the response ref. The v2 side carries the incremental diff worth reviewing: content-root, the manifest-list/root-manifest oneOf, table-location, the metadata-location relaxation, and format-version max 4. v2 operations are suffixed (loadTableV2 etc.) so the v1 operationIds stay unchanged. make lint and make generate both pass.
Removes meta-commentary about the spec document from the descriptions added for
v4 -- phrases describing the schema mechanism rather than the API behavior
("enforced by the oneOf below", "stated normatively", "this is stated
explicitly", "cannot be expressed here because...") and a couple of redundant
justification tails. The normative rules and design rationale are unchanged;
only the wrapping prose is trimmed to match the terser style of the surrounding
spec. No schema or field semantics change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[WIP] PR to drive discussion around the changes needed in the IRC spec for V4 tables.
These changes are needed because:
manifest-list. TheSnapshotmust contain one ofmanifest-listorcontent-rootnow.metadata-locationis now optional because it won't be set for catalog-managed tablestable-location-- metadata can contain relative paths now, this field will be used to specify the absolute path of the table.