-
Notifications
You must be signed in to change notification settings - Fork 370
[lake/iceberg] Implement IcebergLakeCatalog for PK Tables #1372
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
[lake/iceberg] Implement IcebergLakeCatalog for PK Tables #1372
Conversation
b57ff0b
to
69df64b
Compare
@luoyuxia Please help me with the review, whenever you get time! |
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 PR implements IcebergLakeCatalog to add support for Fluss primary key tables in Iceberg lake storage. The implementation provides the infrastructure to create Iceberg tables that correspond to Fluss primary key tables with proper schema conversion and partitioning.
- Adds a new IcebergLakeCatalog class implementing the LakeCatalog interface
- Updates IcebergLakeStorage to return the new catalog implementation instead of throwing UnsupportedOperationException
- Defines Iceberg version property in the Maven POM
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
IcebergLakeStorage.java | Updates createLakeCatalog method to return IcebergLakeCatalog instance |
IcebergLakeCatalog.java | New implementation of LakeCatalog interface for Iceberg integration |
pom.xml | Adds iceberg.version property definition |
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
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.
@MehulBatra Thanks for the pr. Left some comments. PTAL.
Also, please don't forget to add test to cover it.
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
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.
@MehulBatra Thanks for the pr. Left minor comments. PTAL
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Show resolved
Hide resolved
...lake/fluss-lake-iceberg/src/main/java/com/alibaba/fluss/lake/iceberg/IcebergLakeCatalog.java
Outdated
Show resolved
Hide resolved
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.
@MehulBatra Left some comments again. PTAL
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
...lake-iceberg/src/test/java/com/apache/fluss/lake/iceberg/catalog/IcebergLakeCatalogTest.java
Outdated
Show resolved
Hide resolved
@MehulBatra I append a some commit to improve some code, PTAL. If you have no question, I'll merge later. |
Thanks @luoyuxia for the review and feedback! |
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.
Merging......
@MehulBatra Thanks for the great work. I’ll follow up |
Purpose
Add support for IcebergLakeCatalog for fluss primary key tables
Linked issue: close #1337
Brief change log
Tests
In progress
API and Format
In progress
Documentation
In progress