[lake/paimon] Support Altering Lake Table Properties#2799
Conversation
ac22201 to
b4de36e
Compare
b4de36e to
cbc02d7
Compare
|
CC @luoyuxia |
There was a problem hiding this comment.
Pull request overview
Enables altering lake-format-specific table properties (e.g., paimon.*) via ALTER TABLE ... SET/RESET, addressing #2795 by removing the server-side blanket restriction and delegating validation to the lake-format implementation.
Changes:
- Remove server-side validation that rejected altering custom properties prefixed with the configured datalake format (e.g.,
paimon.*). - Add Paimon-side validation to block altering unsettable/immutable Paimon options while allowing changeable Paimon options.
- Update lake Paimon IT coverage to validate both failing (immutable) and succeeding (changeable) Paimon option alterations; adjust Flink IT accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| fluss-server/src/main/java/org/apache/fluss/server/utils/TableDescriptorValidation.java | Removes the blanket restriction on altering datalake-format-prefixed custom properties. |
| fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java | Updates alter-table validation call to match the new server-side validation signature. |
| fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/utils/PaimonConversions.java | Adds validation for ALTER operations on Paimon options (blocks unsettable/immutable keys). |
| fluss-lake/fluss-lake-paimon/src/test/java/org/apache/fluss/lake/paimon/LakeEnabledTableCreateITCase.java | Expands IT coverage for altering Paimon options: immutable should fail, changeable should succeed. |
| fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/catalog/FlinkCatalogITCase.java | Removes the previous assertion around paimon.* alter behavior (needs updated coverage per new behavior). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (PAIMON_UNSETTABLE_OPTIONS.contains(key) | ||
| || CoreOptions.IMMUTABLE_OPTIONS.contains(key)) { | ||
| throw new InvalidConfigException( | ||
| String.format("The Paimon option %s cannot be changed.", key)); |
There was a problem hiding this comment.
validateAlterPaimonOptions receives the converted Paimon key (e.g., paimon.bucket becomes bucket), so the thrown error message will refer to the stripped key. This is inconsistent with create-time validation (which reports the original user-specified key) and can be confusing to users troubleshooting ALTER TABLE failures. Consider including the original Fluss property key in the exception message (or re-attaching the paimon. prefix when applicable) so the error points to exactly what the user set/reset.
| if (PAIMON_UNSETTABLE_OPTIONS.contains(key) | |
| || CoreOptions.IMMUTABLE_OPTIONS.contains(key)) { | |
| throw new InvalidConfigException( | |
| String.format("The Paimon option %s cannot be changed.", key)); | |
| String displayKey = key; | |
| if (!displayKey.startsWith(PAIMON_CONF_PREFIX)) { | |
| displayKey = PAIMON_CONF_PREFIX + displayKey; | |
| } | |
| if (PAIMON_UNSETTABLE_OPTIONS.contains(key) | |
| || CoreOptions.IMMUTABLE_OPTIONS.contains(key)) { | |
| throw new InvalidConfigException( | |
| String.format("The Paimon option %s cannot be changed.", displayKey)); |
| String unSupportedDml6 = | ||
| "alter table test_alter_table_append_only set ('paimon.file.format' = 'orc')"; | ||
| assertThatThrownBy(() -> tEnv.executeSql(unSupportedDml6)) | ||
| .rootCause() | ||
| .isInstanceOf(InvalidConfigException.class) | ||
| .hasMessage( | ||
| "Property 'paimon.file.format' is not supported to alter which is for datalake table."); | ||
|
|
||
| String unSupportedDml7 = | ||
| "alter table test_alter_table_append_only set ('auto-increment.fields' = 'b')"; | ||
| assertThatThrownBy(() -> tEnv.executeSql(unSupportedDml7)) | ||
| assertThatThrownBy(() -> tEnv.executeSql(unSupportedDml6)) | ||
| .rootCause() | ||
| .isInstanceOf(CatalogException.class) | ||
| .hasMessage("The option 'auto-increment.fields' is not supported to alter yet."); |
There was a problem hiding this comment.
This test previously asserted that altering a lake-format-specific option via Flink SQL (e.g. a paimon.* key) fails. With the server-side restriction removed and validation moved into the lake implementation, that behavior should be re-asserted with the new expected outcome/message (and ideally also add a positive case for a changeable paimon.* option). As-is, the Flink SQL path no longer has coverage for ALTER TABLE SET/RESET of lake table properties, which is the main feature of this PR.
luoyuxia
left a comment
There was a problem hiding this comment.
@LiebingYu Thanks for the pr. Left minor comments. PTAL
|
|
||
| private static void validateAlterPaimonOptions(String key) { | ||
| if (PAIMON_UNSETTABLE_OPTIONS.contains(key) | ||
| || CoreOptions.IMMUTABLE_OPTIONS.contains(key)) { |
There was a problem hiding this comment.
bucket.num is not in IMMUTABLE_OPTIONS, but actually, in fluss case, it shouldn't change. Maybe we need to add more to unsupported options.
There was a problem hiding this comment.
Currently, Flink doesn't support to alter bucket.num.
If we support to alter bucket.num in the future, we should change PAIMON_UNSETTABLE_OPTIONS to PAIMON_UNALTERABLE_OPTIONS here (because bucket exists in PAIMON_UNSETTABLE_OPTIONS ). But I think we ok to use PAIMON_UNSETTABLE_OPTIONS here for now?
Purpose
Linked issue: close #2795
Brief change log
Tests
API and Format
Documentation