-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Support custom catalog for iceberg external table #5117
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
Conversation
caneGuy
left a comment
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.
LGTM
fe/fe-core/src/main/java/com/starrocks/catalog/IcebergTable.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/com/starrocks/catalog/IcebergResource.java
Outdated
Show resolved
Hide resolved
|
I have left a comment, rest LGTM. |
fe/fe-core/src/main/java/com/starrocks/catalog/IcebergResource.java
Outdated
Show resolved
Hide resolved
| throw new DdlException(ICEBERG_IMPL + " must be set in properties"); | ||
| } | ||
| try { | ||
| Thread.currentThread().getContextClassLoader().loadClass(catalogImpl); |
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.
This is just to verify whether the jvm has loaded the custom class when create customer iceberg resource? If fe restarts and the loadClass() method will not be called.
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.
Are there other considerations here? In fact, there is no need to restart fe by dynamically loading the custom jar.
|
run starrocks_fe_unittest |
fe/fe-core/src/main/java/com/starrocks/catalog/IcebergTable.java
Outdated
Show resolved
Hide resolved
DorianZheng
left a comment
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.
I have left one comment, rest LGTM
[FE PR Coverage check]😞 fail : 26 / 42 (61.90%) file detail
|
Add support for custom catalog which can be defined by users themselves when creating iceberg external table.
The custom catalog should be in the form of IcebergHiveCatalog, in other words extending BaseMetastoreCatalog and implementing IcebergCatalog. The catalog JAR should be placed into each fe/lib directory, and FE has to be restarted before custom catalog works.
Usage:
```sql
CREATE EXTERNAL RESOURCE "iceberg0"
PROPERTIES (
"type" = "iceberg",
"starrocks.catalog-type"="CUSTOM",
"iceberg.catalog-impl"="{The full class name of custom catalog}"
);
```
Extra config users defined can be added in table properties when executing CREATE EXTERNAL TABLE, see StarRocks#2225.
Signed-off-by: EsoragotoSpirit <wanglichen@starrocks.com>
Signed-off-by: EsoragotoSpirit <wanglichen@starrocks.com> (cherry picked from commit 8c46037) Co-authored-by: 絵空事スピリット <wanglichen@starrocks.com>
Signed-off-by: EsoragotoSpirit <wanglichen@starrocks.com> (cherry picked from commit 8c46037) Co-authored-by: 絵空事スピリット <wanglichen@starrocks.com>
Signed-off-by: EsoragotoSpirit <wanglichen@starrocks.com> (cherry picked from commit 8c46037) Co-authored-by: 絵空事スピリット <wanglichen@starrocks.com>
Signed-off-by: EsoragotoSpirit <wanglichen@starrocks.com> (cherry picked from commit 8c46037) Co-authored-by: 絵空事スピリット <wanglichen@starrocks.com>
What type of PR is this:
Which issues of this PR fixes :
Fixes #3826
Problem Summary(Required) :
Add support for custom catalog which can be defined by users themselves when creating iceberg external table.
The custom catalog should be in the form of IcebergHiveCatalog, in other words extending BaseMetastoreCatalog and implementing IcebergCatalog. The catalog JAR should be placed into each fe/lib directory, and FE has to be restarted before custom catalog works.
Usage:
Extra config users defined can be added in table properties when executing CREATE EXTERNAL TABLE, see #2225.