-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-28961: Respect partition limit in alter_table_req for partitioned tables #5823
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
4637790
HIVE-28961: Respect partition limit in alter_table_req for partitione…
wecharyu 72084ce
fix tests
wecharyu eefb427
fix tests
wecharyu 9f38e61
Merge remote-tracking branch 'community/master' into HIVE-28961
wecharyu 42760b3
fix tests
wecharyu 5e6f26e
Merge remote-tracking branch 'community/master' into HIVE-28961
wecharyu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,8 +54,10 @@ | |
import org.apache.hadoop.hive.metastore.api.MetaException; | ||
import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; | ||
import org.apache.hadoop.hive.metastore.api.Partition; | ||
import org.apache.hadoop.hive.metastore.api.PartitionsRequest; | ||
import org.apache.hadoop.hive.metastore.api.Table; | ||
import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants; | ||
import org.apache.thrift.TException; | ||
|
||
import java.io.IOException; | ||
import java.net.URI; | ||
|
@@ -351,26 +353,30 @@ public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbnam | |
String oldTblLocPath = srcPath.toUri().getPath(); | ||
String newTblLocPath = dataWasMoved ? destPath.toUri().getPath() : null; | ||
|
||
// also the location field in partition | ||
parts = msdb.getPartitions(catalogName, databaseName, tableName, -1); | ||
for (Partition part : parts) { | ||
String oldPartLoc = part.getSd().getLocation(); | ||
if (dataWasMoved && oldPartLoc.contains(oldTblLocPath)) { | ||
URI oldUri = new Path(oldPartLoc).toUri(); | ||
String newPath = oldUri.getPath().replace(oldTblLocPath, newTblLocPath); | ||
Path newPartLocPath = new Path(oldUri.getScheme(), oldUri.getAuthority(), newPath); | ||
part.getSd().setLocation(newPartLocPath.toString()); | ||
} | ||
part.setDbName(newDbName); | ||
part.setTableName(newTblName); | ||
} | ||
// Do not verify stats parameters on a partitioned table. | ||
msdb.alterTable(catalogName, databaseName, tableName, newt, null); | ||
int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), | ||
MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); | ||
|
||
// alterPartition is only for changing the partition location in the table rename | ||
if (dataWasMoved) { | ||
PartitionsRequest req = new PartitionsRequest(newDbName, newTblName); | ||
req.setCatName(catName); | ||
req.setMaxParts((short) -1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a brief comment explaining the rationale behind using -1 to indicate fetching all partitions to aid future maintainers. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
parts = handler.get_partitions_req(req).getPartitions(); | ||
|
||
for (Partition part : parts) { | ||
deniskuzZ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
String oldPartLoc = part.getSd().getLocation(); | ||
if (oldPartLoc.contains(oldTblLocPath)) { | ||
URI oldUri = new Path(oldPartLoc).toUri(); | ||
String newPath = oldUri.getPath().replace(oldTblLocPath, newTblLocPath); | ||
Path newPartLocPath = new Path(oldUri.getScheme(), oldUri.getAuthority(), newPath); | ||
part.getSd().setLocation(newPartLocPath.toString()); | ||
} | ||
part.setDbName(newDbName); | ||
part.setTableName(newTblName); | ||
} | ||
|
||
Batchable.runBatched(partitionBatchSize, parts, new Batchable<Partition, Void>() { | ||
@Override | ||
public List<Void> run(List<Partition> input) throws Exception { | ||
|
@@ -410,7 +416,10 @@ public List<Void> run(List<Partition> input) throws Exception { | |
msdb.alterTable(catalogName, databaseName, tableName, newt, null); | ||
|
||
if (cascade || retainOnColRemoval) { | ||
parts = msdb.getPartitions(catalogName, databaseName, tableName, -1); | ||
PartitionsRequest req = new PartitionsRequest(dbname, name); | ||
req.setCatName(catName); | ||
req.setMaxParts((short) -1); | ||
deniskuzZ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
parts = handler.get_partitions_req(req).getPartitions(); | ||
Table finalOldt = oldt; | ||
int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), | ||
MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); | ||
|
@@ -469,17 +478,13 @@ public List<Void> run(List<Partition> input) throws Exception { | |
} | ||
// commit the changes | ||
success = msdb.commitTransaction(); | ||
} catch (InvalidObjectException e) { | ||
} catch (InvalidOperationException | MetaException e) { | ||
throw e; | ||
} catch (TException e) { | ||
LOG.debug("Failed to get object from Metastore ", e); | ||
throw new InvalidOperationException( | ||
"Unable to change partition or table." | ||
+ " Check metastore logs for detailed stack." + e.getMessage()); | ||
} | ||
catch (NoSuchObjectException e) { | ||
LOG.debug("Object not found in metastore ", e); | ||
throw new InvalidOperationException( | ||
"Unable to change partition or table. Object " + e.getMessage() + " does not exist." | ||
+ " Check metastore logs for detailed stack."); | ||
} finally { | ||
if (success) { | ||
// Txn was committed successfully. | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please give more context about this change.
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
dataWasMoved
is false, we do not need get all partitions and update the location of partitions.