-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-3107]Fix HiveSyncTool drop partitions using JDBC or hivesql or hms #4453
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
|
|
||
| public StringBuilder getAlterTableDropPrefix(String tableName) { | ||
| StringBuilder alterSQL = new StringBuilder("ALTER TABLE "); | ||
| alterSQL.append(HIVE_ESCAPE_CHARACTER).append(config.databaseName) |
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'm thinking, drop partition is not a high-frequency action, is it easier to understand using String.format? Although the performance of StringBuilder is the best.
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.
ahhh, keep as it is :>
| public static void createReplaceCommit(String instantTime, String partitions) | ||
| throws IOException { | ||
| HoodieReplaceCommitMetadata replaceCommitMetadata = new HoodieReplaceCommitMetadata(); | ||
| replaceCommitMetadata.setOperationType(WriteOperationType.DELETE_PARTITION); |
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.
Here as a method to pass parameters?
createReplaceCommit(String instantTime, String partitions) -> createReplaceCommit(String instantTime, String partitions, WriteOperationType OperationType)
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.
Sure thing, Changed. Thanks a lot for your review.
|
hi @zhangyue19921010 This pr is very good overall, thank you very much. |
| } | ||
| return String.join("/", partBuilder); | ||
| } | ||
|
|
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 method is basically the same as the method in HMSDDLExecutor.java . could we abstract out a unified method?
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.
Okay, Changed.
|
hi @XuQianJin-Stars and @xiarixiaoyao Thanks a lot for your review. Comments are addressed. Could you please take a look? Thanks a lot. |
|
@zhangyue19921010 @xiarixiaoyao : looks like there is a consistent CI failure after merging this patch https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build?definitionId=3&_a=summary |
|
stacktrace from my local run in intellij. |
|
I tried to fix the issue, but guess its involved. with this fix, hiveql succeeds, but hms and jdbc fails. |
|
@nsivabalan on it. |
|
The root cause of Here are the related stack trace Just raise a pr to fix the test with adding the schema info in fake replace commit file. |
|
thanks for the quick turn around. I merged the patch. |
Thanks a lot for your review. It's my pleasure. |
…hms (#4453) * constructDropPartitions when drop partitions using jdbc * done * done * code style * code review Co-authored-by: yuezhang <[email protected]>
…hms (apache#4453) * constructDropPartitions when drop partitions using jdbc * done * done * code style * code review Co-authored-by: yuezhang <[email protected]>
Fix HiveSyncTool drop partitions function didn't work as expected.
Because we may build sql for jdbc or use hms api wrong.
Take jdbc as example :
Current Sql
ALTER TABLE forecast_agg DROP PARTITION (20210623/0/20210623)Correct SQL
Executing SQL ALTER TABLE yz.forecast_agg DROP IF EXISTS PARTITION (forecast_predict_date='20210623',timezone='0',date='20210623')Just add a UT named
testDropPartitionto explain this bug in detailsWithout this patch, UT will failed throws exception mentioned below