-
Notifications
You must be signed in to change notification settings - Fork 970
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
[core] Support add partition for paimon #4323
Conversation
@@ -273,6 +273,17 @@ default void invalidateTable(Identifier identifier) {} | |||
void dropPartition(Identifier identifier, Map<String, String> partitions) | |||
throws TableNotExistException, PartitionNotExistException; | |||
|
|||
/** | |||
* Create a partition for the specify table. |
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.
specified
@@ -310,6 +310,27 @@ private Map<String, String> convertToProperties(Database database) { | |||
return 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.
* @throws PartitionNotExistException if the partition does not exist | ||
*/ | ||
void createPartition(Identifier identifier, Map<String, String> partitions) | ||
throws TableNotExistException, PartitionNotExistException; |
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.
When throw PartitionNotExistException?
Identifier identifier = toIdentifier(tablePath); | ||
catalog.createPartition(identifier, partitionSpec.getPartitionSpec()); | ||
} else { | ||
throw new UnsupportedOperationException(); |
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.
message?
} catch (Catalog.TableNotExistException e) { | ||
throw new CatalogException(e); | ||
} catch (Catalog.PartitionNotExistException e) { | ||
throw new RuntimeException(e); |
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 throw RuntimeException?
TableSchema tableSchema = getDataTableSchema(identifier); | ||
if (!tableSchema.partitionKeys().isEmpty() | ||
&& new CoreOptions(tableSchema.options()).partitionedTableInMetastore()) { | ||
try { |
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.
Add a warn log if tableSchema.partitionKeys().isEmpty() or tableSchema.options()).partitionedTableInMetastore() is false?
catalog = ((CachingCatalog) catalog).wrapped(); | ||
} | ||
|
||
if (catalog instanceof HiveCatalog) { |
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 need to check HiveCatalog?
} | ||
} | ||
try { | ||
if (catalog instanceof CachingCatalog) { |
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 need to check CachingCatalog?
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.
|
||
if (catalog instanceof HiveCatalog) { | ||
Identifier identifier = toIdentifier(tablePath); | ||
catalog.createPartition(identifier, partitionSpec.getPartitionSpec()); |
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.
Just invoke this?
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.
Had move the logic to AbstractCatalog, so Catalog API not contains createPartition API.
throws TableNotExistException { | ||
TableSchema tableSchema = getDataTableSchema(identifier); | ||
if (!tableSchema.partitionKeys().isEmpty() | ||
&& new CoreOptions(tableSchema.options()).partitionedTableInMetastore()) { |
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.
throw Unsupport exception if it is not partitioned table in metastore.
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.
Done
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 think you should use MetastoreClient
to add partition. The implementation can be in AbstractCatalog
.
Yes, move to AbstractCatalog maybe better, would change as it. Thanks @JingsongLi |
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.
+1
Purpose
Currently paimon can not support add partition, due to add partition not need commit transaction,so make hms sync to achieve it.
Linked issue: close #xxx
Tests
API and Format
Documentation