[SPARK-22787] [TEST] [SQL] Add a TPC-H query suite #19982

gatorsmile · 2017-12-14T19:54:40Z

What changes were proposed in this pull request?

Add a test suite to ensure all the TPC-H queries can be successfully analyzed, optimized and compiled without hitting the max iteration threshold.

How was this patch tested?

N/A

gatorsmile · 2017-12-14T19:55:30Z

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

+  private def readToUnsafeMem(
+      conf: Broadcast[SerializableConfiguration],
+      requiredSchema: StructType,
+      wholeTextMode: Boolean): (PartitionedFile) => Iterator[UnsafeRow] = {


This is just a style fix of a PR I merged today.

gatorsmile · 2017-12-14T19:56:40Z

cc @cloud-fan @maropu @kiszk

SparkQA · 2017-12-14T19:59:39Z

Test build #84927 has finished for PR 19982 at commit 9d52ff0.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2017-12-14T20:17:52Z

sql/core/src/test/resources/tpch/q1.sql

+-- using default substitutions
+
+select
+	l_returnflag,


Can we use space instead of tab?

This is the generated SQL. I plan to keep them unchanged.

SparkQA · 2017-12-14T22:48:49Z

Test build #84928 has finished for PR 19982 at commit fac5fb2.

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

maropu · 2017-12-14T23:39:55Z

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

+    "q1", "q2", "q3", "q4", "q5", "q6", "q7", "q8", "q9", "q10", "q11",
+    "q12", "q13", "q14", "q15", "q16", "q17", "q18", "q19", "q20", "q21", "q22")
+
+  private def checkGeneratedCode(plan: SparkPlan): Unit = {


How about making a base trait to remove duplicate code between this and TPCDSQuerySuite?

maropu · 2017-12-14T23:41:15Z

LGTM for one minor comment.
BTW, how about also adding star schema benchmark (many join cases) to check compilation?

gatorsmile · 2017-12-14T23:48:35Z

https://www.cs.umb.edu/~poneil/StarSchemaB.PDF This one?

maropu · 2017-12-14T23:56:12Z

yea.

maropu · 2017-12-14T23:57:55Z

~~Probably, we could get sql queries from here https://github.com/electrum/ssb-dbgen (I don't look into yet though...)~~ Probably, we need to get sql queries from the paper (or, https://github.com/eyalroz/ssb-tools/tree/master/benchmark_queries)

kiszk · 2017-12-15T01:08:47Z

LGTM

viirya · 2017-12-15T01:25:48Z

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

+  /**
+   * Drop all the tables
+   */
+  protected override def afterAll(): Unit = {


As same as TPCDSQuerySuite, we can use BenchmarkQueryTest.afterAll.

yea we don't need to overwrite it here.

viirya · 2017-12-15T01:30:58Z

LGTM with one minor comment.

maropu · 2017-12-15T01:45:35Z

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

+  protected override def afterAll(): Unit = {
+    try {
+      // For debugging dump some statistics about how much time was spent in various optimizer rules
+      logWarning(RuleExecutor.dumpTimeSpent())


Why warning for debug uses?

Also, is that a bad idea to dump the time for each query?

If we do use logWarning, the messages will not be shown in the test log.

This is just to give the overall picture how long each rule takes.

I plan to submit another PR to track which rule takes an effect for a specific query and also record the time cost.

SparkQA · 2017-12-15T03:08:36Z

Test build #84936 has finished for PR 19982 at commit 9dc4457.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
abstract class BenchmarkQueryTest extends QueryTest with SharedSQLContext with BeforeAndAfterAll
class TPCDSQuerySuite extends BenchmarkQueryTest
class TPCHQuerySuite extends BenchmarkQueryTest

gatorsmile · 2017-12-15T04:13:52Z

cc @maropu Feel free to submit a PR for adding SSB

maropu · 2017-12-15T04:20:05Z

yea, sure. I have much bandwidth now :)

SparkQA · 2017-12-15T06:47:06Z

Test build #84940 has finished for PR 19982 at commit 2671416.

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

gatorsmile · 2017-12-15T06:57:15Z

Thanks! Merged to master.

maropu · 2017-12-15T10:58:58Z

#19990

maropu · 2017-12-23T01:38:28Z

@gatorsmile Any progress on this? #19982 (comment)
After I thought your comment, I came up with collecting metrics for each rule like;
master...maropu:MetricSpike
This conflicts with your activity, or this is not acceptable? welcome any comment.

gatorsmile · 2017-12-24T06:37:59Z

@maropu Thanks for your contribution. It looks over engineering. We do not need such complicated solutions for this simple use case. We just need to record them in the log. We are also proposing new APIs for our logs. @jiangxb1987 is working on the design.

maropu · 2017-12-24T08:57:37Z

ok, I just look forward to the proposal.

fix.

9d52ff0

gatorsmile commented Dec 14, 2017

View reviewed changes

gatorsmile mentioned this pull request Dec 14, 2017

[SPARK-22774][SQL][Test] Add compilation check into TPCDSQuerySuite #19971

Closed

gatorsmile added 2 commits December 14, 2017 12:08

fix.

a5f1957

fix.

fac5fb2

dongjoon-hyun reviewed Dec 14, 2017

View reviewed changes

maropu reviewed Dec 14, 2017

View reviewed changes

address comments.

9dc4457

viirya reviewed Dec 15, 2017

View reviewed changes

maropu reviewed Dec 15, 2017

View reviewed changes

address comments.

2671416

asfgit closed this in 3fea5c4 Dec 15, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-22787] [TEST] [SQL] Add a TPC-H query suite #19982

[SPARK-22787] [TEST] [SQL] Add a TPC-H query suite #19982

gatorsmile commented Dec 14, 2017

gatorsmile Dec 14, 2017

gatorsmile commented Dec 14, 2017

SparkQA commented Dec 14, 2017

dongjoon-hyun Dec 14, 2017

gatorsmile Dec 14, 2017

SparkQA commented Dec 14, 2017

maropu Dec 14, 2017

maropu commented Dec 14, 2017

gatorsmile commented Dec 14, 2017

maropu commented Dec 14, 2017

maropu commented Dec 14, 2017 •

edited

Loading

kiszk commented Dec 15, 2017

viirya Dec 15, 2017

cloud-fan Dec 15, 2017

viirya commented Dec 15, 2017

maropu Dec 15, 2017

maropu Dec 15, 2017

gatorsmile Dec 15, 2017

gatorsmile Dec 15, 2017

maropu Dec 15, 2017

SparkQA commented Dec 15, 2017

gatorsmile commented Dec 15, 2017

maropu commented Dec 15, 2017

SparkQA commented Dec 15, 2017

gatorsmile commented Dec 15, 2017

maropu commented Dec 15, 2017

maropu commented Dec 23, 2017

gatorsmile commented Dec 24, 2017

maropu commented Dec 24, 2017

[SPARK-22787] [TEST] [SQL] Add a TPC-H query suite #19982

[SPARK-22787] [TEST] [SQL] Add a TPC-H query suite #19982

Conversation

gatorsmile commented Dec 14, 2017

What changes were proposed in this pull request?

How was this patch tested?

Choose a reason for hiding this comment

gatorsmile commented Dec 14, 2017

SparkQA commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 14, 2017

Choose a reason for hiding this comment

maropu commented Dec 14, 2017

gatorsmile commented Dec 14, 2017

maropu commented Dec 14, 2017

maropu commented Dec 14, 2017 • edited Loading

kiszk commented Dec 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viirya commented Dec 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 15, 2017

gatorsmile commented Dec 15, 2017

maropu commented Dec 15, 2017

SparkQA commented Dec 15, 2017

gatorsmile commented Dec 15, 2017

maropu commented Dec 15, 2017

maropu commented Dec 23, 2017

gatorsmile commented Dec 24, 2017

maropu commented Dec 24, 2017

maropu commented Dec 14, 2017 •

edited

Loading