OpenAI: IRC spec changes for v4 [WIP][Discuss] - #17543
Draft
dhruvarya-db wants to merge 2 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
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.