-
Notifications
You must be signed in to change notification settings - Fork 486
[lake/paimon] Compare paimon schema and Fluss schema before alter table. #2331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request enhances the Paimon lake catalog integration by comparing Paimon and Fluss schemas before altering tables. This prevents schema inconsistencies that can occur when a previous alter operation partially succeeds (e.g., Paimon succeeds but Fluss fails).
Key Changes:
- Added
getFlussSchema()method toLakeCatalog.Contextinterface to provide current Fluss schema during alter operations - Implemented schema validation logic in
PaimonLakeCatalogto detect and handle schema mismatches between Paimon and Fluss - Updated
MetadataManagerandCoordinatorServiceto pass schema information via the context
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| fluss-common/src/main/java/org/apache/fluss/lake/lakestorage/LakeCatalog.java | Added getFlussSchema() method to Context interface |
| fluss-common/src/test/java/org/apache/fluss/lake/lakestorage/TestingLakeCatalogContext.java | Updated test context to support schema parameter |
| fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorService.java | Updated to create context with schema, and pass FlussPrincipal instead of pre-created context |
| fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java | Changed method signatures to accept FlussPrincipal and create context locally with current schema |
| fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java | Added schema comparison logic to detect and handle retry scenarios |
| fluss-lake/fluss-lake-paimon/src/test/java/org/apache/fluss/lake/paimon/PaimonLakeCatalogTest.java | Enhanced tests to cover schema mismatch scenarios and retry logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java
Outdated
Show resolved
Hide resolved
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java
Outdated
Show resolved
Hide resolved
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/org/apache/fluss/lake/lakestorage/LakeCatalog.java
Outdated
Show resolved
Hide resolved
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/org/apache/fluss/lake/lakestorage/LakeCatalog.java
Outdated
Show resolved
Hide resolved
| TableChange tableChange = tableChanges.get(i); | ||
| if (!(tableChange instanceof TableChange.AddColumn | ||
| && ((TableChange.AddColumn) tableChange).getPosition() | ||
| == TableChange.ColumnPosition.last() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could perform this check earlier—it’s not related to the existing Paimon schema. Also, it’s already validated here:
fluss/fluss-server/src/main/java/org/apache/fluss/server/coordinator/SchemaUpdate.java
Lines 101 to 104 in 256bd7f
| TableChange.ColumnPosition position = addColumn.getPosition(); | |
| if (position != TableChange.ColumnPosition.last()) { | |
| throw new IllegalArgumentException("Only support addColumn column at last now."); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not related to the existing Paimon schema
If paimon schema is same as fluss schema, all the operations(both add column and set config) can be accepted. Otherwise, only add columns can be accepted to make the schema same.
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/PaimonLakeCatalog.java
Outdated
Show resolved
Hide resolved
dcc741c to
fdec0b8
Compare
Purpose
Linked issue: close #2330
Brief change log
Tests
API and Format
Documentation