[core] Mark table 'type' option as immutable#8564
Open
plusplusjiajia wants to merge 1 commit into
Open
Conversation
c96949b to
959dcaf
Compare
The 'type' option decides which table kind the catalog builds
(CatalogUtils reads it on every load), but it was missing from
IMMUTABLE_OPTIONS, so ALTER TABLE SET ('type'='format-table') on an
existing table succeeded. The next load then builds a different table
kind (e.g. FormatTable) over the existing FileStore data layout: data
files under bucket directories are no longer readable, snapshot history
is orphaned, and -- since only FileStore tables wire up query auth --
an authorized table silently stops enforcing row filters and column
masks.
Changing a table's type in place has no supported semantics: there are
no tests or docs exercising it, and replaceTable() already handles
cross-type replacement explicitly via drop-and-create. Mark TYPE
@immutable so the checkAlterTableOption / checkResetTableOption /
dynamic-option guards reject it, consistent with the other structural
options (primary-key, partition, merge-engine, ...).
Unlike the other immutable options, the type is also rejected without
snapshots: table kinds like format tables never create Paimon snapshots
but can still hold data, so the snapshot-based emptiness check does not
apply to it. Since a table created without an explicit type is of the
default type, isUnchangedNormalizedKey treats setting the default
explicitly as unchanged, so redundant 'type'='table' dynamic options
and idempotent ALTERs still pass. Type values are compared
case-insensitively, matching OptionsUtils.convertToEnum, so 'type'='TABLE'
-- and a case-only restatement of an explicitly stored type -- is also
recognized as unchanged.
The Flink dynamic-option preflight
(AbstractFlinkTableFactory.buildPaimonTable) diffed the raw option maps
and called checkAlterTableOption without that normalization, so once
'type' became immutable a documented
paimon.<catalog>.<db>.<table>.type=table dynamic option on an
implicit-type table would hit the new guard and throw. It now applies
isUnchangedNormalizedKey as well -- via a List-keyed overload that takes
the primary/partition keys directly, since only the schema (not a full
TableSchema) is available there. For a FormatCatalogTable, whose
getOptions() drops Paimon's 'type', the wrapped table's effective
format-table type is restored before the check so a no-op
'type'='format-table' dynamic option is not treated as a change either.
Real type changes are still rejected.
959dcaf to
813a470
Compare
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.
Purpose
typewas missing fromIMMUTABLE_OPTIONS, soALTER TABLE SET ('type'='format-table')on a table with data succeeded. The next load then builds a different table kind over the existing FileStore layout:data files become unreadable, snapshot history is orphaned, and query auth (row filters / column masks) is silently bypassed since only FileStore tables wire it up.
In-place type change has no supported semantics (no tests/docs;
replaceTable()already does cross-type via drop-and-create), so markTYPE@Immutablelike the other structural options.Tests
SchemaManagerTest#testAlterImmutableOptionsOnEmptyTable:SET 'type'on a table with snapshots now throwsChange 'type' is not supported yet.API and Format
No format change.
ALTER SET/RESET 'type'on a table with snapshots (previously undefined behavior) is now rejected.