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
[SPARK-22214][SQL] Refactor the list hive partitions code #19444
Conversation
@@ -638,12 +638,14 @@ private[hive] class HiveClientImpl( | |||
table: CatalogTable, | |||
spec: Option[TablePartitionSpec]): Seq[CatalogTablePartition] = withHiveState { | |||
val hiveTable = toHiveTable(table, Some(userName)) | |||
val parts = spec match { | |||
case None => shim.getAllPartitions(client, hiveTable).map(fromHivePartition) |
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.
After this change, HiveShim.getAllPartitions
is only used to support HiveShim.getPartitionsByFilter
for hive 0.12, we may consider completely remove the method in the future.
Test build #82509 has finished for PR 19444 at commit
|
@@ -638,12 +638,14 @@ private[hive] class HiveClientImpl( | |||
table: CatalogTable, | |||
spec: Option[TablePartitionSpec]): Seq[CatalogTablePartition] = withHiveState { | |||
val hiveTable = toHiveTable(table, Some(userName)) | |||
val parts = spec match { | |||
case None => shim.getAllPartitions(client, hiveTable).map(fromHivePartition) | |||
val partialPartSpec = spec match { |
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.
-> partSpec
LGTM except a minor comment. |
LGTM |
Test build #82519 has finished for PR 19444 at commit
|
Thanks! Merged to master. |
/** | ||
* Initialize an empty spec. | ||
*/ | ||
lazy val emptyTablePartitionSpec: TablePartitionSpec = Map.empty[String, String] |
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.
Map.empty
is already an object, I think we can jus inline it
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.
We wanted to refer the val emptyTablePartitionSpec
as TablePartitionSpec
, not Map[String, String]
, though they are equal.
What changes were proposed in this pull request?
In this PR we make a few changes to the list hive partitions code, to make the code more extensible.
The following changes are made:
HiveClientImpl.getPartitions()
, callclient.getPartitions
instead ofshim.getAllPartitions
whenspec
is empty;HiveTableScanExec
, previously we always calllistPartitionsByFilter
if the configmetastorePartitionPruning
is enabled, but actually, we'd better calllistPartitions
ifpartitionPruningPred
is empty;HiveTableScanExec
.How was this patch tested?
Tested by existing test cases since this is code refactor, no regression or behavior change is expected.