[SPARK-30525][SQL]HiveTableScanExec do not need to prune partitions again after pushing down to SessionCatalog for partition pruning by fuwhu · Pull Request #27232 · apache/spark

fuwhu · 2020-01-16T08:21:30Z

What changes were proposed in this pull request?

HiveTableScanExec does not prune partitions again after SessionCatalog.listPartitionsByFilter called.

Why are the changes needed?

In HiveTableScanExec, it will push down to hive metastore for partition pruning if spark.sql.hive.metastorePartitionPruning is true, and then it will prune the returned partitions again using partition filters, because some predicates, eg. "b like 'xyz'", are not supported in hive metastore. But now this problem is already fixed in HiveExternalCatalog.listPartitionsByFilter, the HiveExternalCatalog.listPartitionsByFilter can return exactly what we want now. So it is not necessary any more to double prune in HiveTableScanExec.

Does this PR introduce any user-facing change?

no

How was this patch tested?

Existing unit tests.

SparkQA · 2020-01-16T10:58:47Z

Test build #116829 has finished for PR 27232 at commit 6abfdc9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-16T16:30:57Z

Test build #116856 has finished for PR 27232 at commit ef469f5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AngersZhuuuu · 2020-01-17T02:23:16Z

emmmm, prune partition again is because when call listPartitionsByFilter kit can't convert all spark filter expression to hive metastore filter condition. and will get more partition then exactly wanted. then prune again will spark's method to keep smallest partitions.

If we can't promise all these case was fixed in listPartitionsByFilter, we may still need this. and it's cost is negligible

fuwhu · 2020-01-17T15:00:09Z

emmmm, prune partition again is because when call listPartitionsByFilter kit can't convert all spark filter expression to hive metastore filter condition. and will get more partition then exactly wanted. then prune again will spark's method to keep smallest partitions.

If we can't promise all these case was fixed in listPartitionsByFilter, we may still need this. and it's cost is negligible

HiveExternalCatalog.listPartitionsByFilter will call HiveClient.getPartitionsByFilter to push down to hive metastore for partition pruning, which may not convert all spark filters to hive filters.

But now it already call ExternalCatalogUtils.prunePartitionsByFilter to prune the results returned by HiveClient.getPartitionsByFilter again in HiveExternalCatalog.listPartitionsByFilter. So it is not necessary any more to prune again in HiveTableScanExec.

you can check the code : https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala#L1254

fuwhu · 2020-01-18T01:30:57Z

cc @cloud-fan

SparkQA · 2020-01-20T08:05:02Z

Test build #117093 has finished for PR 27232 at commit ef469f5.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

fuwhu · 2020-01-21T04:22:40Z

retest this please.

cloud-fan · 2020-01-21T06:45:34Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala

can the tests call partitions?

The test is to verify the SQLConf HIVE_METASTORE_PARTITION_PRUNING, but the partitions method will return same result no matter HIVE_METASTORE_PARTITION_PRUNING is true or false.

what about just remove the test case "Verify SQLConf HIVE_METASTORE_PARTITION_PRUNING" in HiveTableScanSuite?

updated to refine the test, please help review again. thanks a lot.

SparkQA · 2020-01-21T06:46:01Z

Test build #117152 has finished for PR 27232 at commit ef469f5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-21T10:22:56Z

Test build #117165 has finished for PR 27232 at commit d8eefe9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-28T04:11:52Z

Test build #117457 has finished for PR 27232 at commit 95d62c4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-28T04:26:20Z

Test build #117462 has finished for PR 27232 at commit ff7d71a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-28T12:36:02Z

Test build #117480 has finished for PR 27232 at commit df9c4e1.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2020-01-29T02:32:49Z

I'm not a good reviewer for this

fuwhu · 2020-01-29T10:12:34Z

I'm not a good reviewer for this

ok, sorry for bothering.

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala

SparkQA · 2020-01-31T17:59:45Z

Test build #117664 has finished for PR 27232 at commit a862bd5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

…ionsByFilter called in HiveTableScanExec.

SparkQA · 2020-02-01T05:17:56Z

Test build #117708 has finished for PR 27232 at commit 057a594.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

fuwhu · 2020-02-02T08:52:55Z

cc @cloud-fan

cloud-fan · 2020-02-03T04:16:42Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala

+        prunePartitions(hivePartitions)
+      }
+    } else {
+      if (sparkSession.sessionState.conf.metastorePartitionPruning) {


to be consistent, we should add && partitionPruningPred.nonEmpty

sure, added.

cloud-fan · 2020-02-03T04:17:42Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala

+
  // exposed for tests
-  @transient lazy val rawPartitions = {
+  @transient lazy val rawPartitions: Seq[HivePartition] = {


when we call rawPartitions, the relation.prunedPartitions must be empty. We can remove relation.prunedPartitions.getOrElse below.

sure, removed.

cloud-fan · 2020-02-03T04:18:06Z

LGTM except 2 minor comments

cloud-fan · 2020-02-03T07:27:42Z

retest this please

fuwhu · 2020-02-03T08:03:22Z

retest this please

cloud-fan · 2020-02-03T11:47:51Z

retest this please

SparkQA · 2020-02-03T14:04:50Z

Test build #117772 has finished for PR 27232 at commit f0ad037.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-02-03T17:25:08Z

thanks, merging to master!

fuwhu · 2020-02-04T01:51:20Z

thank you all for help.

fuwhu force-pushed the SPARK-30525 branch from 6abfdc9 to ef469f5 Compare January 16, 2020 13:59

fuwhu closed this Jan 20, 2020

fuwhu reopened this Jan 20, 2020

cloud-fan reviewed Jan 21, 2020

View reviewed changes

fuwhu force-pushed the SPARK-30525 branch from ef469f5 to d8eefe9 Compare January 21, 2020 07:53

fuwhu force-pushed the SPARK-30525 branch from d8eefe9 to 95d62c4 Compare January 28, 2020 01:49

fuwhu requested review from cloud-fan and wangyum and removed request for wangyum January 28, 2020 02:30

fuwhu requested a review from srowen January 29, 2020 02:28

srowen removed their request for review January 29, 2020 02:32

fuwhu changed the title ~~[SPARK-30525][SQL]HiveTableScanExec do not need to prune partitions again after pushing down to hive metastore~~ [SPARK-30525][SQL]HiveTableScanExec do not need to prune partitions again Jan 30, 2020

fuwhu requested review from gatorsmile, gengliangwang and maropu and removed request for maropu January 30, 2020 04:08

fuwhu force-pushed the SPARK-30525 branch from df9c4e1 to a862bd5 Compare January 31, 2020 15:35

cloud-fan reviewed Jan 31, 2020

View reviewed changes

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala Outdated Show resolved Hide resolved

fuwhu added 6 commits February 1, 2020 09:00

Do not need to prune partitions again after SessionCatalog.listPartit…

837dc48

…ionsByFilter called in HiveTableScanExec.

Refine code.

6cfa4ab

refine code.

f86671d

fix unit test failure.

d96f19b

refine code

5bc37c3

refine code

057a594

fuwhu force-pushed the SPARK-30525 branch from a862bd5 to 057a594 Compare February 1, 2020 02:14

fuwhu changed the title ~~[SPARK-30525][SQL]HiveTableScanExec do not need to prune partitions again~~ [SPARK-30525][SQL]HiveTableScanExec do not need to prune partitions again after pushing down to SessionCatalog for partition pruning Feb 1, 2020

cloud-fan reviewed Feb 3, 2020

View reviewed changes

refine code

f0ad037

cloud-fan closed this in 47659a0 Feb 3, 2020

fuwhu deleted the SPARK-30525 branch February 4, 2020 01:50

dongjoon-hyun added the SQL label Feb 5, 2020

Conversation

fuwhu commented Jan 16, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

AngersZhuuuu commented Jan 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fuwhu commented Jan 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fuwhu commented Jan 18, 2020

Uh oh!

SparkQA commented Jan 20, 2020

Uh oh!

fuwhu commented Jan 21, 2020

Uh oh!

cloud-fan Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 21, 2020

Uh oh!

SparkQA commented Jan 21, 2020

Uh oh!

SparkQA commented Jan 28, 2020

Uh oh!

SparkQA commented Jan 28, 2020

Uh oh!

SparkQA commented Jan 28, 2020

Uh oh!

srowen commented Jan 29, 2020

Uh oh!

fuwhu commented Jan 29, 2020

Uh oh!

Uh oh!

SparkQA commented Jan 31, 2020

Uh oh!

SparkQA commented Feb 1, 2020

Uh oh!

fuwhu commented Feb 2, 2020

Uh oh!

cloud-fan Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Feb 3, 2020

Uh oh!

cloud-fan commented Feb 3, 2020

Uh oh!

fuwhu commented Feb 3, 2020

Uh oh!

cloud-fan commented Feb 3, 2020

Uh oh!

SparkQA commented Feb 3, 2020

Uh oh!

cloud-fan commented Feb 3, 2020

Uh oh!

fuwhu commented Feb 4, 2020

Uh oh!

Reviewers

AngersZhuuuu commented Jan 17, 2020 •

edited

Loading

fuwhu commented Jan 17, 2020 •

edited

Loading