-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Feature] Support glue hive metastore #12250
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.
I wonder why you put glue under hive directory. I think they are on the same directory level.
Because we use the AWS glue data catalog as the metastore for Hive here, it's just have special "hive.metastore.type" which is "glue", but it still need use hive connector/catalog to get meta info |
* a) Hides the non-metastore related operations present in AWSGlue | ||
* b) Hides away the batching and pagination related limitations of AWSGlue | ||
*/ | ||
public interface AWSGlueMetastore { |
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.
Why not reuse IHiveMetastore
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 interface is AWSCatalogMetastoreClient's internal, and AWSCatalogMetastoreClient is the client of IHiveMetastore(such like HiveMetaStore). This interface should retrun the meta info of glue instead of the meta info of SR.
run starrocks_fe_unittest |
starrocks_be_unittest succeeded. |
starrocks_be_unittest succeeded. |
starrocks_be_unittest succeeded. |
starrocks_be_unittest succeeded. |
starrocks_be_unittest succeeded. |
run starrocks_admit_test |
@@ -35,7 +30,7 @@ public DeltaLakeInternalMgr(String catalogName, Map<String, String> properties) | |||
|
|||
public IHiveMetastore createHiveMetastore() { | |||
// TODO(stephen): Abstract the creator class to construct hive meta client | |||
HiveMetaClient metaClient = createHiveMetaClient(); | |||
HiveMetaClient metaClient = HiveMetaClient.createHiveMetaClient(properties); |
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.
My origin idea was to construct the hive conf in an external component.
fe/fe-core/src/main/java/com/starrocks/external/hive/HiveMetastoreApiConverter.java
Show resolved
Hide resolved
@@ -79,11 +87,17 @@ public static HiveTable toHiveTable(Table table, String catalogName) { | |||
.setFullSchema(toFullSchemasForHiveTable(table)) | |||
.setTableLocation(table.getSd().getLocation()) | |||
.setCreateTime(table.getCreateTime()); | |||
|
|||
Map<String, String> tableParams = table.getParameters() == null ? ImmutableMap.of() : table.getParameters(); | |||
if (isDeltaLakeTable(tableParams)) { |
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.
abstract a function named 'toTableLocation()'. we may try our best to keep clear here.
properties.forEach(conf::set); | ||
if (!properties.containsKey(HIVE_METASTORE_URIS)) { | ||
// set value for compatible with the rollback | ||
properties.put(HIVE_METASTORE_URIS, DUMMY_THRIFT_URI); |
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.
why use this ? The properties must have "hive.metastore.uris".
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.
glue doesn't have this property
starrocks_be_unittest succeeded. |
starrocks_be_unittest succeeded. |
run starrocks_admit_test |
starrocks_be_unittest succeeded. |
SonarCloud Quality Gate failed. |
run starrocks_fe_unittest |
run starrocks_admit_test |
run starrocks_fe_unittest |
1 similar comment
run starrocks_fe_unittest |
[FE PR Coverage Check]😞 fail : 26 / 2204 (01.18%) file detail
|
What type of PR is this:
Which issues of this PR fixes :
#12249
In This PR, we support create hive type catalog whcih metasotre type is glue, you can create the catalog like this
If the data file is stored in S3, also need S3 configuration, refer to https://docs.starrocks.io/zh-cn/latest/using_starrocks/External_table#aws-s3tencent-cloud-cos%E6%94%AF%E6%8C%81
Problem Summary(Required) :
Checklist: