Core: Validate format version compatibility client-side on REST table create - #17728
Open
nssalian wants to merge 1 commit into
Open
Core: Validate format version compatibility client-side on REST table create#17728nssalian wants to merge 1 commit into
nssalian wants to merge 1 commit into
Conversation
| return newTableMetadata(schema, spec, SortOrder.unsorted(), location, properties); | ||
| } | ||
|
|
||
| public static void checkFormatVersionCompatibility( |
Contributor
There was a problem hiding this comment.
I don't think I'd add this public utility, at least for now, until we really have multiple callers, and know what appropriate input would be for a whole "formatVersionCompatibility" check. Right now it's just doing schema and properties, and Schema.checkCompatibility is doing the heavy lifting, and the properties is only used to extract the format version.
Think I'd reccomend just inlining this above in the RestSessionCatalog#create call
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
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.
Rationale for the change
#17500 added RCK coverage for variant columns following up with the client side change here.
The REST client previously forwarded the
CreateTableRequestto the server without validating the schema against the requested format version. So the rejection of a v3-only type (variant, timestamp_ns, geometry/geography, unknown, non-null defaults) on a lower format version depended entirely on the server, and the server's error message is not spec-defined. That is why the RCK cannot assert on it.Non-REST catalogs already run this check locally via
TableMetadata.newTableMetadata->Builder.build()->Schema.checkCompatibility. This PR makes the REST client do the same, so the failure and its message are the client's, before any request is sent.Changes
TableMetadata.checkFormatVersionCompatibility(schema, properties): resolves the requested format version (default v2) and runs the existingSchema.checkCompatibility. TheDEFAULT_TABLE_FORMAT_VERSIONconstant stays package-private.RESTSessionCatalog.create()andstageCreate(): call it before building theCreateTableRequest, so an incompatible schema is rejected client-side with the well-defined"... is not supported until v3"message and no request is issued.TestRESTCatalog.testCreateV2TableWithVariantColumnFailsClientSide: asserts theIllegalStateExceptionmessage AND that no create-tablePOSTreaches the server (the assertion that fails if the client-side check is removed).Notes
replaceTransaction) already buildTableMetadataclient-side and are unaffected.CatalogTests.testCreateV2TableWithVariantColumnFailscontinues to cover the message/behavior; the new REST-specific test adds the client-side-rejection guarantee.