[SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data #29384

maropu · 2020-08-07T10:04:55Z

What changes were proposed in this pull request?

TPCDSQuerySuite currently computes plans with empty TPCDS tables, then checks if plans can be generated correctly. But, the generated plans can be different from actual ones because the input tables are empty (e.g., the plans always use broadcast-hash joins, but actual ones use sort-merge joins for larger tables). To mitigate the issue, this PR defines data statistics constants extracted from generated TPCDS data in TPCDSTableStats, then injects the statistics via spark.sessionState.catalog.alterTableStats when defining TPCDS tables in TPCDSQuerySuite.

Please see a link below about how to extract the table statistics:

https://gist.github.com/maropu/f553d32c323ee803d39e2f7fa0b5a8c3

For example, the generated plans of TPCDS q2 are different with/without this fix:

==== w/ this fix: q2 ====
== Physical Plan ==
* Sort (43)
+- Exchange (42)
   +- * Project (41)
      +- * SortMergeJoin Inner (40)
         :- * Sort (28)
         :  +- Exchange (27)
         :     +- * Project (26)
         :        +- * BroadcastHashJoin Inner BuildRight (25)
         :           :- * HashAggregate (19)
         :           :  +- Exchange (18)
         :           :     +- * HashAggregate (17)
         :           :        +- * Project (16)
         :           :           +- * BroadcastHashJoin Inner BuildRight (15)
         :           :              :- Union (9)
         :           :              :  :- * Project (4)
         :           :              :  :  +- * Filter (3)
         :           :              :  :     +- * ColumnarToRow (2)
         :           :              :  :        +- Scan parquet default.web_sales (1)
         :           :              :  +- * Project (8)
         :           :              :     +- * Filter (7)
         :           :              :        +- * ColumnarToRow (6)
         :           :              :           +- Scan parquet default.catalog_sales (5)
         :           :              +- BroadcastExchange (14)
         :           :                 +- * Project (13)
         :           :                    +- * Filter (12)
         :           :                       +- * ColumnarToRow (11)
         :           :                          +- Scan parquet default.date_dim (10)
         :           +- BroadcastExchange (24)
         :              +- * Project (23)
         :                 +- * Filter (22)
         :                    +- * ColumnarToRow (21)
         :                       +- Scan parquet default.date_dim (20)
         +- * Sort (39)
            +- Exchange (38)
               +- * Project (37)
                  +- * BroadcastHashJoin Inner BuildRight (36)
                     :- * HashAggregate (30)
                     :  +- ReusedExchange (29)
                     +- BroadcastExchange (35)
                        +- * Project (34)
                           +- * Filter (33)
                              +- * ColumnarToRow (32)
                                 +- Scan parquet default.date_dim (31)

==== w/o this fix: q2 ====
== Physical Plan ==
* Sort (40)
+- Exchange (39)
   +- * Project (38)
      +- * BroadcastHashJoin Inner BuildRight (37)
         :- * Project (26)
         :  +- * BroadcastHashJoin Inner BuildRight (25)
         :     :- * HashAggregate (19)
         :     :  +- Exchange (18)
         :     :     +- * HashAggregate (17)
         :     :        +- * Project (16)
         :     :           +- * BroadcastHashJoin Inner BuildRight (15)
         :     :              :- Union (9)
         :     :              :  :- * Project (4)
         :     :              :  :  +- * Filter (3)
         :     :              :  :     +- * ColumnarToRow (2)
         :     :              :  :        +- Scan parquet default.web_sales (1)
         :     :              :  +- * Project (8)
         :     :              :     +- * Filter (7)
         :     :              :        +- * ColumnarToRow (6)
         :     :              :           +- Scan parquet default.catalog_sales (5)
         :     :              +- BroadcastExchange (14)
         :     :                 +- * Project (13)
         :     :                    +- * Filter (12)
         :     :                       +- * ColumnarToRow (11)
         :     :                          +- Scan parquet default.date_dim (10)
         :     +- BroadcastExchange (24)
         :        +- * Project (23)
         :           +- * Filter (22)
         :              +- * ColumnarToRow (21)
         :                 +- Scan parquet default.date_dim (20)
         +- BroadcastExchange (36)
            +- * Project (35)
               +- * BroadcastHashJoin Inner BuildRight (34)
                  :- * HashAggregate (28)
                  :  +- ReusedExchange (27)
                  +- BroadcastExchange (33)
                     +- * Project (32)
                        +- * Filter (31)
                           +- * ColumnarToRow (30)
                              +- Scan parquet default.date_dim (29)

This comes from the @cloud-fan comment: #29270 (comment)

Why are the changes needed?

For better test coverage.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

maropu · 2020-08-07T10:06:07Z

sql/core/src/test/scala/org/apache/spark/sql/TPCDSTableStats.scala

+  // These data statistics are extracted from generated TPCDS data with SF=1
+
+  // scalastyle:off line.size.limit
+  val sf1TableStats = Map(


I will replace this with larger data case, e.g., SF=100.

maropu · 2020-08-07T12:46:30Z

cc: @Ngone51 @cloud-fan @viirya

SparkQA · 2020-08-07T14:54:20Z

Test build #127197 has finished for PR 29384 at commit 3b03d70.

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

dongjoon-hyun

It's great, @maropu !
+1, LGTM. Merged to master!

dongjoon-hyun · 2020-08-07T15:18:34Z

@maropu . Can we have this nice patch in branch-3.0, too?

viirya

lgtm too

cloud-fan · 2020-08-07T16:15:24Z

late LGTM. Shall we run the test twice with 100T stats and empty tables?

SparkQA · 2020-08-07T17:23:52Z

Test build #127205 has finished for PR 29384 at commit b345248.

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

maropu · 2020-08-07T23:15:20Z

Thanks, all!

@dongjoon-hyun Sure!
@cloud-fan okay, I'll do follow-up.

… generation on actual TPCDS data ### What changes were proposed in this pull request? `TPCDSQuerySuite` currently computes plans with empty TPCDS tables, then checks if plans can be generated correctly. But, the generated plans can be different from actual ones because the input tables are empty (e.g., the plans always use broadcast-hash joins, but actual ones use sort-merge joins for larger tables). To mitigate the issue, this PR defines data statistics constants extracted from generated TPCDS data in `TPCDSTableStats`, then injects the statistics via `spark.sessionState.catalog.alterTableStats` when defining TPCDS tables in `TPCDSQuerySuite`. Please see a link below about how to extract the table statistics: - https://gist.github.com/maropu/f553d32c323ee803d39e2f7fa0b5a8c3 For example, the generated plans of TPCDS `q2` are different with/without this fix: ``` ==== w/ this fix: q2 ==== == Physical Plan == * Sort (43) +- Exchange (42) +- * Project (41) +- * SortMergeJoin Inner (40) :- * Sort (28) : +- Exchange (27) : +- * Project (26) : +- * BroadcastHashJoin Inner BuildRight (25) : :- * HashAggregate (19) : : +- Exchange (18) : : +- * HashAggregate (17) : : +- * Project (16) : : +- * BroadcastHashJoin Inner BuildRight (15) : : :- Union (9) : : : :- * Project (4) : : : : +- * Filter (3) : : : : +- * ColumnarToRow (2) : : : : +- Scan parquet default.web_sales (1) : : : +- * Project (8) : : : +- * Filter (7) : : : +- * ColumnarToRow (6) : : : +- Scan parquet default.catalog_sales (5) : : +- BroadcastExchange (14) : : +- * Project (13) : : +- * Filter (12) : : +- * ColumnarToRow (11) : : +- Scan parquet default.date_dim (10) : +- BroadcastExchange (24) : +- * Project (23) : +- * Filter (22) : +- * ColumnarToRow (21) : +- Scan parquet default.date_dim (20) +- * Sort (39) +- Exchange (38) +- * Project (37) +- * BroadcastHashJoin Inner BuildRight (36) :- * HashAggregate (30) : +- ReusedExchange (29) +- BroadcastExchange (35) +- * Project (34) +- * Filter (33) +- * ColumnarToRow (32) +- Scan parquet default.date_dim (31) ==== w/o this fix: q2 ==== == Physical Plan == * Sort (40) +- Exchange (39) +- * Project (38) +- * BroadcastHashJoin Inner BuildRight (37) :- * Project (26) : +- * BroadcastHashJoin Inner BuildRight (25) : :- * HashAggregate (19) : : +- Exchange (18) : : +- * HashAggregate (17) : : +- * Project (16) : : +- * BroadcastHashJoin Inner BuildRight (15) : : :- Union (9) : : : :- * Project (4) : : : : +- * Filter (3) : : : : +- * ColumnarToRow (2) : : : : +- Scan parquet default.web_sales (1) : : : +- * Project (8) : : : +- * Filter (7) : : : +- * ColumnarToRow (6) : : : +- Scan parquet default.catalog_sales (5) : : +- BroadcastExchange (14) : : +- * Project (13) : : +- * Filter (12) : : +- * ColumnarToRow (11) : : +- Scan parquet default.date_dim (10) : +- BroadcastExchange (24) : +- * Project (23) : +- * Filter (22) : +- * ColumnarToRow (21) : +- Scan parquet default.date_dim (20) +- BroadcastExchange (36) +- * Project (35) +- * BroadcastHashJoin Inner BuildRight (34) :- * HashAggregate (28) : +- ReusedExchange (27) +- BroadcastExchange (33) +- * Project (32) +- * Filter (31) +- * ColumnarToRow (30) +- Scan parquet default.date_dim (29) ``` This comes from the cloud-fan comment: #29270 (comment) This is the backport of #29384. ### Why are the changes needed? For better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests. Closes #29390 from maropu/SPARK-32564-BRANCH3.0. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

…ty tables ### What changes were proposed in this pull request? This is the follow-up PR of #29384 to address the cloud-fan comment: #29384 (comment) This PR re-enables `TPCDSQuerySuite` with empty tables for better test coverages. ### Why are the changes needed? For better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests. Closes #29391 from maropu/SPARK-32564-FOLLOWUP. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

…ty tables ### What changes were proposed in this pull request? This is the follow-up PR of #29384 to address the cloud-fan comment: #29384 (comment) This PR re-enables `TPCDSQuerySuite` with empty tables for better test coverages. ### Why are the changes needed? For better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests. Closes #29391 from maropu/SPARK-32564-FOLLOWUP. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 1df855b) Signed-off-by: Dongjoon Hyun <[email protected]>

Ngone51 · 2020-08-10T12:53:52Z

@maropu Thank you for the great work! Now #29270 can reuse the statistics as well.

Fix

3b03d70

probot-autolabeler bot added the SQL label Aug 7, 2020

maropu commented Aug 7, 2020

View reviewed changes

maropu mentioned this pull request Aug 7, 2020

[SPARK-32466][TEST][SQL] Add PlanStabilitySuite to detect SparkPlan regression #29270

Closed

Update stats based on SF=100

b345248

maropu changed the title ~~[WIP][SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data~~ [SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data Aug 7, 2020

dongjoon-hyun approved these changes Aug 7, 2020

View reviewed changes

dongjoon-hyun closed this in 5b8444a Aug 7, 2020

viirya reviewed Aug 7, 2020

View reviewed changes

This was referenced Aug 8, 2020

[SPARK-32564][SQL][TEST][3.0] Inject data statistics to simulate plan generation on actual TPCDS data #29390

Closed

[SPARK-32564][SQL][TEST][FOLLOWUP] Re-enable TPCDSQuerySuite with empty tables #29391

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data #29384

[SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data #29384

Uh oh!

maropu commented Aug 7, 2020

Uh oh!

maropu Aug 7, 2020

Uh oh!

maropu commented Aug 7, 2020

Uh oh!

SparkQA commented Aug 7, 2020

Uh oh!

dongjoon-hyun left a comment

Uh oh!

dongjoon-hyun commented Aug 7, 2020

Uh oh!

viirya left a comment

Uh oh!

cloud-fan commented Aug 7, 2020

Uh oh!

SparkQA commented Aug 7, 2020

Uh oh!

maropu commented Aug 7, 2020 •

edited

Loading

Uh oh!

Ngone51 commented Aug 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data #29384

[SPARK-32564][SQL][TEST] Inject data statistics to simulate plan generation on actual TPCDS data #29384

Uh oh!

Conversation

maropu commented Aug 7, 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!

maropu Aug 7, 2020

Choose a reason for hiding this comment

Uh oh!

maropu commented Aug 7, 2020

Uh oh!

SparkQA commented Aug 7, 2020

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Aug 7, 2020

Uh oh!

viirya left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Aug 7, 2020

Uh oh!

SparkQA commented Aug 7, 2020

Uh oh!

maropu commented Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ngone51 commented Aug 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maropu commented Aug 7, 2020 •

edited

Loading