[HUDI-6671] Support 'alter table add partition' sql#9408
[HUDI-6671] Support 'alter table add partition' sql#9408danny0405 merged 4 commits intoapache:masterfrom
Conversation
| } | ||
| if (enableHiveStylePartitioning) { | ||
| partitionColumn + "=" + encodedPartitionValue | ||
| } else { |
There was a problem hiding this comment.
Why removing the handling of hive style partitioning?
There was a problem hiding this comment.
Not remove it, just reuse the common implementation of makePartitionPath which will handle hive style and url encode.
| override def run(sparkSession: SparkSession): Seq[Row] = { | ||
| val fullTableName = s"${tableIdentifier.database}.${tableIdentifier.table}" | ||
| logInfo(s"start execute alter table add partition command for $fullTableName") | ||
|
|
There was a problem hiding this comment.
Do we check the existence of the table itself ?
There was a problem hiding this comment.
Spark analysis will throw exception if table does not exist, so we do not need check it here.
| class TestAlterTableAddPartition extends HoodieSparkSqlTestBase { | ||
|
|
||
| test("Add partition for non-partitioned table") { | ||
| val tableName = generateTableName |
There was a problem hiding this comment.
Use function withTable to clean it
|
|
||
| override def run(sparkSession: SparkSession): Seq[Row] = { | ||
| val fullTableName = s"${tableIdentifier.database}.${tableIdentifier.table}" | ||
| logInfo(s"start execute alter table add partition command for $fullTableName") |
There was a problem hiding this comment.
Can directly use tableIdentifier here since it already override the toString method(which is formatted)
|
|
||
| // Sync new partitions in batch, enable ignoreIfExists to avoid sync failure. | ||
| val batchSize = sparkSession.sparkContext.conf.getInt("spark.sql.addPartitionInBatch.size", 100) | ||
| parts.toIterator.grouped(batchSize).foreach { batch => |
There was a problem hiding this comment.
Should we try catch here? Looks CreateHoodieTableCommand catch exception if syncing to HMS occurs error.
There was a problem hiding this comment.
ping @danny0405 any thoughts for this? I see some commands catch the exception and only print warning(like CreateHoodieTableCommand), and some commands throw the exception out(like DropHoodieTableCommand, CreateHoodieTableAsSelectCommand)
Looks we don't have any standards whether throw exceptions if syncing to HMS occurs error.
There was a problem hiding this comment.
I have no preference, if we throw exception for hive sync, will the table created be cleaned ?
There was a problem hiding this comment.
Like CreateHoodieTableAsSelectCommand, it would clean the created table, but some commands don't guarantee this.
Will print warning for AddPartitionCommand, looks it quite difficult to rollback partitions added.
There was a problem hiding this comment.
It makes sense to add a warning log, otherwise the exception will be displayed in client while we succeed add partition folder actually.
|
@hudi-bot run azure |
|
@hudi-bot run azure |
|
@hudi-bot run azure |
1 similar comment
|
@hudi-bot run azure |
4 participants
Change Logs
Hoodie does not support 'add partition' sql now, so we can not get partitions added by 'add partition' command.
In this patch, we implement add partition in Hoodie side:
AlterHoodieTableAddPartitionCommandTestAlterTableAddPartitionImpact
No
Risk level (write none, low medium or high below)
Low
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist