[SPARK-30516][SQL] involve partition filters in the statistic estimation of FileScan #27213

fuwhu · 2020-01-15T07:02:35Z

What changes were proposed in this pull request?

Refine FileScan.estimateStatistics to take partitionFilters into account.

Why are the changes needed?

Currently, FileScan.estimateStatistics does not take partitionFilters into account, which may lead to bigger sizeInBytes. It should be reasonable to change it to involve partitionFilters when estimating the statistics.

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing unit tests.

SparkQA · 2020-01-15T08:05:01Z

Test build #116754 has finished for PR 27213 at commit a5a3012.

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

fuwhu · 2020-01-16T08:25:37Z

retest this please.

fuwhu · 2020-01-16T08:39:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

This config is proposed in #27129 , will resolve the conflict after #27129 finished.

How about marking this PR as WIP until #27129 is merged?

sure, thanks.

fuwhu · 2020-01-16T08:49:05Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileIndex.scala

Since dataFilters is not used in all sub classes of FileIndex, so just remove the dataFilters parameter here.

@fuwhu I would suggest keep the current method with parameter dataFilters.
See the discussions in #27157
Also, the rename is not related to the proposal of this PR, right?

yea, just changed it since i thought this method return sequence of PartitionDirectory objects, which is not actually to list files.
Will change it back to keep the PR proposal clear.

@gengliangwang
I removed the conf MAX_PARTITION_NUMBER_FOR_STATS_CALCULATION_VIA_FS in this PR, since it is not needed per discussion in #27129 .

SparkQA · 2020-01-16T12:47:26Z

Test build #116830 has finished for PR 27213 at commit a5a3012.

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

SparkQA · 2020-01-16T13:45:04Z

Test build #116837 has finished for PR 27213 at commit d7485ce.

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

SparkQA · 2020-01-16T13:58:11Z

Test build #116839 has finished for PR 27213 at commit d8949ba.

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

SparkQA · 2020-01-28T06:56:58Z

Test build #117463 has finished for PR 27213 at commit d68b50f.

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

SparkQA · 2020-01-30T19:20:38Z

Test build #117571 has finished for PR 27213 at commit 7790a88.

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

fuwhu · 2020-01-31T00:52:30Z

retest this please

SparkQA · 2020-01-31T05:41:13Z

Test build #117601 has finished for PR 27213 at commit 7790a88.

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

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

Test build #117621 has finished for PR 27213 at commit e948650.

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

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

Test build #117618 has finished for PR 27213 at commit c29feb1.

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

fuwhu · 2020-01-31T12:00:04Z

retest this please

SparkQA · 2020-01-31T18:45:10Z

Test build #117652 has finished for PR 27213 at commit e948650.

This patch fails from timeout after a configured wait of 400m.
This patch merges cleanly.
This patch adds no public classes.

fuwhu · 2020-02-01T00:53:01Z

retest this please

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

Test build #117702 has finished for PR 27213 at commit e948650.

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

fuwhu · 2020-02-04T01:54:57Z

cc @cloud-fan

fuwhu · 2020-02-04T02:30:25Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/InMemoryFileIndex.scala

Just add an assertion incidentally here, this is not necessary for this PR.
if not ok, i can remove it.

SparkQA · 2020-02-04T04:09:55Z

Test build #117799 has finished for PR 27213 at commit 86b9043.

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

fuwhu · 2020-02-04T05:30:03Z

retest this please

SparkQA · 2020-02-04T05:33:24Z

Test build #117803 has started for PR 27213 at commit 86b9043.

SparkQA · 2020-02-04T06:15:58Z

Test build #117797 has finished for PR 27213 at commit 95d59a3.

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

maropu · 2020-02-10T01:12:47Z

Could you add some tests for this improvement?

… statistic sizeInBytes in FileScan.estimateStatistics

…s equal to the root paths of patitions specified by userSpecifiedPartitionSpec.

and MAX_PARTITION_NUMBER_FOR_STATS_CALCULATION_VIA_FS into account.

…onf.

fuwhu · 2020-02-10T09:04:49Z

Could you add some tests for this improvement?

@maropu Added one test, please help review. thanks.

SparkQA · 2020-02-10T14:08:33Z

Test build #118147 has finished for PR 27213 at commit 2e210a0.

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

fuwhu · 2020-02-11T22:23:01Z

cc @cloud-fan

github-actions · 2020-05-22T00:13:36Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

fuwhu commented Jan 16, 2020

View reviewed changes

fuwhu force-pushed the SPARK-30516 branch from d7485ce to d8949ba Compare January 16, 2020 09:00

fuwhu force-pushed the SPARK-30516 branch from d8949ba to d68b50f Compare January 28, 2020 02:45

fuwhu requested review from cloud-fan and gengliangwang January 28, 2020 02:47

fuwhu changed the title ~~[SPARK-30516][SQL] statistic estimation of FileScan should take partitionFilters and partition number into account~~ [SPARK-30516][SQL][WIP] statistic estimation of FileScan should take partitionFilters and partition number into account Jan 30, 2020

fuwhu force-pushed the SPARK-30516 branch from c29feb1 to e948650 Compare January 31, 2020 05:25

fuwhu changed the title ~~[SPARK-30516][SQL][WIP] statistic estimation of FileScan should take partitionFilters and partition number into account~~ [SPARK-30516][SQL] statistic estimation of FileScan should take partitionFilters and partition number into account Jan 31, 2020

fuwhu changed the title ~~[SPARK-30516][SQL] statistic estimation of FileScan should take partitionFilters and partition number into account~~ [SPARK-30516][SQL] statistic estimation of FileScan should take partitionFilters into account Jan 31, 2020

fuwhu removed request for cloud-fan and gengliangwang February 4, 2020 01:54

fuwhu force-pushed the SPARK-30516 branch from e948650 to 95d59a3 Compare February 4, 2020 02:23

fuwhu commented Feb 4, 2020

View reviewed changes

dongjoon-hyun added the SQL label Feb 5, 2020

fuwhu added 10 commits February 10, 2020 10:15

Take partitionFilters and partition num into account when calculating…

b889eb4

… statistic sizeInBytes in FileScan.estimateStatistics

Add assert to make sure the rootPathsSpecified of InMemoryFileIndex i…

7d0cbd3

…s equal to the root paths of patitions specified by userSpecifiedPartitionSpec.

Refine FileScan.estimateStatistics to take partitionFilters

0646d9a

and MAX_PARTITION_NUMBER_FOR_STATS_CALCULATION_VIA_FS into account.

Fix compile error.

c4f0c19

roll back on the change on method FileIndex.listFiles

7c2c3ca

Remove check on partition number when estmating statistics in FileScan.

a443031

remove conf MAX_PARTITION_NUMBER_FOR_STATS_CALCULATION_VIA_FS in SQLC…

56f7efa

…onf.

refine code

be7e660

refine code

94ea426

Add test in FileBasedDataSourceSuite

2e210a0

fuwhu force-pushed the SPARK-30516 branch from 86b9043 to 2e210a0 Compare February 10, 2020 08:58

fuwhu changed the title ~~[SPARK-30516][SQL] statistic estimation of FileScan should take partitionFilters into account~~ [SPARK-30516][SQL] involve partition filters in the statistic estimation of FileScan Feb 11, 2020

github-actions bot added the Stale label May 22, 2020

github-actions bot closed this May 23, 2020

[SPARK-30516][SQL] involve partition filters in the statistic estimation of FileScan #27213

[SPARK-30516][SQL] involve partition filters in the statistic estimation of FileScan #27213

Uh oh!

Conversation

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

Uh oh!

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 15, 2020

Uh oh!

fuwhu commented Jan 16, 2020

Uh oh!

fuwhu Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

gengliangwang Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

fuwhu Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

gengliangwang Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

SparkQA commented Jan 28, 2020

Uh oh!

SparkQA commented Jan 30, 2020

Uh oh!

fuwhu commented Jan 31, 2020

Uh oh!

SparkQA commented Jan 31, 2020

Uh oh!

SparkQA commented Jan 31, 2020

Uh oh!

SparkQA commented Jan 31, 2020

Uh oh!

fuwhu commented Jan 31, 2020

Uh oh!

SparkQA commented Jan 31, 2020

Uh oh!

fuwhu commented Feb 1, 2020

Uh oh!

SparkQA commented Feb 1, 2020

Uh oh!

fuwhu commented Feb 4, 2020

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 4, 2020

Uh oh!

fuwhu commented Feb 4, 2020

Uh oh!

SparkQA commented Feb 4, 2020

Uh oh!

SparkQA commented Feb 4, 2020

Uh oh!

maropu commented Feb 10, 2020

Uh oh!

fuwhu commented Jan 15, 2020 •

edited

Loading

fuwhu Jan 30, 2020 •

edited

Loading

fuwhu Jan 31, 2020 •

edited

Loading

fuwhu Feb 4, 2020 •

edited

Loading

fuwhu commented Feb 10, 2020 •

edited

Loading