-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Fix creating non-bucketed empty partition #12119
Conversation
Fix a bug where creating empty parition using CALL statement throws exceptions when using file based metastore implementation.
@@ -1220,6 +1220,13 @@ public HiveInsertTableHandle beginInsert(ConnectorSession session, ConnectorTabl | |||
Map<List<String>, ComputedStatistics> partitionComputedStatistics = createComputedStatisticsToPartitionMap(computedStatistics, partitionedBy, columnTypes); | |||
|
|||
for (PartitionUpdate partitionUpdate : partitionUpdates) { | |||
if (partitionUpdate.getFileNames().size() == 0) { |
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.
Use isEmpty()
@@ -1220,6 +1220,13 @@ public HiveInsertTableHandle beginInsert(ConnectorSession session, ConnectorTabl | |||
Map<List<String>, ComputedStatistics> partitionComputedStatistics = createComputedStatisticsToPartitionMap(computedStatistics, partitionedBy, columnTypes); | |||
|
|||
for (PartitionUpdate partitionUpdate : partitionUpdates) { | |||
if (partitionUpdate.getFileNames().size() == 0) { | |||
HiveWriteUtils.createDirectory( |
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.
Static import
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 don't think creating the directory here is there correct place. We should be doing it in SemiTransactionalHiveMetastore
as part of the commit. We probably also need to handle it in the rollback cleaup. @haozhun should take a look
" FORMAT = 'ORC', " + | ||
" partitioned_by = ARRAY['p_varchar'] " + | ||
") " + | ||
"AS " + |
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.
Creating an empty table should be sufficient. We should be able to create a new table and then add an empty partition. Is there a reason why another partition was needed, or something you wanted test?
assertUpdate(format("" + | ||
"CREATE TABLE %s " + | ||
"WITH ( " + | ||
" FORMAT = 'ORC', " + |
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.
Nit: lowercase the name format
and use consistent indentation with the other property (two spaces)
Superseded by #12204 |
Fix a bug where creating empty parition using CALL statement
throws exceptions when using file based metastore implementation.
#12002