-
Notifications
You must be signed in to change notification settings - Fork 335
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
[#1974] fix(hive): Hive table listPartitions reduces HMS RPC calls #1975
Conversation
return IntStream.range(0, partitionNames.size()) | ||
.mapToObj(i -> fromHivePartition(partitionNames.get(i), partitions.get(i))) | ||
List<String> partCols = | ||
table.buildPartitionKeys().stream().map(FieldSchema::getName).collect(Collectors.toList()); |
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 the partition keys of the underlying Hive table were changed, table.buildPartitionKeys()
will get an error result.
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.
Hive table partition keys will not change unless the table is dropped and then created.
.map( | ||
partition -> | ||
fromHivePartition( | ||
FileUtils.makePartName(partCols, partition.getValues()), 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.
Is it necessary to ensure that partCols
and partition.getValues()
have corresponding orders?
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.
@@ -41,7 +41,7 @@ public class TestHiveTableOperations extends MiniHiveMetastoreService { | |||
private static Partition existingPartition; | |||
|
|||
@BeforeAll | |||
private static void setup() { | |||
public static void setup() { |
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 change the access modifier to public
?
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.
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.
LGTM. Could you please review this when you have time? @jerryshao @FANNG1 Thank you!
LGTM |
…1975) ### What changes were proposed in this pull request? Use HMS's `listPartitions` RPC to replace the two RPC calls of `listPartitionNames` and `getPartitionsByNames`. ### Why are the changes needed? Fix: #1974 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? exist UT
What changes were proposed in this pull request?
Use HMS's
listPartitions
RPC to replace the two RPC calls oflistPartitionNames
andgetPartitionsByNames
.Why are the changes needed?
Fix: #1974
Does this PR introduce any user-facing change?
No
How was this patch tested?
exist UT