[SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite #14625

gatorsmile · 2016-08-12T19:11:03Z

What changes were proposed in this pull request?

#14498 plans to remove Hive Built-in Hash Functions. 10+ test cases are broken because the results are different from the Hive golden answer files. These broken test cases are not Hive specific. Thus, it makes more sense to move them to SQLQueryTestSuite

Based on file-based SQL end-to-end testing framework in SQLQueryTestSuite, this PR is to create test cases for joins in general. We will try to move most Join-specific SQL test cases into the new joins.sql file.

How was this patch tested?

This PR is just for improving test cases.

gatorsmile · 2016-08-12T19:12:45Z

cc @rxin @petermaxlee @cloud-fan @hvanhovell

Before moving all the test cases of auto joins, I want to confirm whether this is the right direction. Thanks!

rxin · 2016-08-12T19:17:56Z

sql/core/src/test/resources/sql-tests/inputs/auto_join0.sql

@@ -0,0 +1,9 @@
+select sum(hash(a.k1,a.v1,a.k2, a.v2))


we don't guarantee hash consistency across versions so I'm not sure if it is a good idea to use hash here.

rxin · 2016-08-12T19:19:10Z

What exactly is the data set testing? I'm not a big fan of just pulling a non-trivial (say beyond 5 records) dataset in. It makes it very difficult to understand what exactly the test case is testing, and what the correct output should be.

gatorsmile · 2016-08-12T20:46:20Z

The original purpose of Hive test sets is for verifying the query results and behaviors of Auto Join Conversion

This is not applicable to Spark SQL. I assume the current purposes in Spark are just for checking whether the query results match the outputs of Hive.

Do you want me to completely remove them? Or reduce the data set to around 5 records and keep the queries untouched?

rxin · 2016-08-12T20:50:47Z

I think we should create a comprehensive test suite for joins, and that should use a small dataset.

The Hive thing I'm OK with keeping it for a while, but eventually we should remove them since we are not even sure what they are testing ... I don't remember a case in which these join tests in Hive caught an issue that was not caught by our regular join suites.

SparkQA · 2016-08-12T20:59:06Z

Test build #63703 has finished for PR 14625 at commit 9888f8a.

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

gatorsmile · 2016-08-12T21:25:05Z

I completely agree. Now, if we still want to temporarily keep these test cases, what should we do next? Based on my understanding, we want to get rid of TestHiveSessionState, TestHiveQueryExecution, TestHiveFunctionRegistry, ...

To remove these TestHive-specific classes, should we just use the existing SQLQuerySuite.scala and create a separate folder for these test cases and data sets? When it is ready, we can easily remove all of them in the future. Thanks!

cloud-fan · 2016-08-13T02:43:29Z

I think HiveCompatibilitySuite has 2 purposes:

using Hive's test to improve our test coverage
make sure our result is same with Hive for same query(with blacklist)

Now we have SQLQueryTestSuite, we should use it to improve test coverage instead of HiveCompatibilitySuite. But this doesn't mean we should copy the test from HiveCompatibilitySuite to SQLQueryTestSuite, a test with hundreds of input records doesn't make sense.

HiveCompatibilitySuite will be simplified a lot and only contains tests just for compatibility checking.

gatorsmile · 2016-08-13T04:00:36Z

@cloud-fan I see. Will try to rewrite the test cases with very few records. Thank you!

SparkQA · 2016-08-13T08:39:24Z

Test build #63716 has finished for PR 14625 at commit 239191c.

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

SparkQA · 2016-08-13T08:50:11Z

Test build #63717 has finished for PR 14625 at commit 5f22316.

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

SparkQA · 2016-08-13T18:40:59Z

Test build #63730 has finished for PR 14625 at commit 3fe55f1.

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

rxin · 2016-08-13T20:19:49Z

@gatorsmile the comment should apply not only to data, but also query (e.g. what case we are testing ...)

gatorsmile · 2016-08-13T21:54:46Z

@rxin Sure, will do it.

gatorsmile · 2016-08-13T22:08:21Z

FYI, found the original JIRA that delivered the first 25 auto_join test cases to Hive: https://issues.apache.org/jira/browse/HIVE-1642

gatorsmile · 2016-08-13T23:29:46Z

Below is the output of Hive for the same queries. They are the same.

outputHive.txt

rxin · 2016-08-13T23:39:30Z

Can we repurpose this ticket to just create test cases for joins in general?

gatorsmile · 2016-08-14T00:44:06Z

Sure. The scope is a little bit large, but let me try to go over the existing join-related test cases in the test suites. We might not be able to cover all of them in a single ticket. Will try my best.

SparkQA · 2016-08-14T02:19:41Z

Test build #63737 has finished for PR 14625 at commit cdea1a3.

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

SparkQA · 2016-08-18T00:46:49Z

Test build #63945 has finished for PR 14625 at commit 7c6f85a.

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

SparkQA · 2016-08-18T01:28:27Z

Test build #63947 has finished for PR 14625 at commit 4bd38d2.

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

SparkQA · 2016-08-18T02:07:03Z

Test build #63950 has finished for PR 14625 at commit a059c77.

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

SparkQA · 2016-08-18T03:01:11Z

Test build #63957 has finished for PR 14625 at commit b4801e0.

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

gatorsmile · 2016-08-18T03:32:27Z

@rxin @cloud-fan The code is ready for review. Thanks!

SparkQA · 2016-08-20T05:58:19Z

Test build #64120 has finished for PR 14625 at commit bf55624.

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

cloud-fan · 2016-08-21T02:46:01Z

sql/core/src/test/resources/sql-tests/inputs/join.sql

@@ -0,0 +1,225 @@
+-- join nested table expressions (auto_join0.q)


Do we need to reference to the hive .q file? I think hive golden file tests will be removed eventually.

: ) That is for helping reviewers know the origins of the queries. If you think we do not care, we can remove it.

SparkQA · 2016-08-22T05:40:09Z

Test build #64174 has finished for PR 14625 at commit 9c6be8e.

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

SparkQA · 2016-08-22T07:56:40Z

Test build #64181 has finished for PR 14625 at commit e2677da.

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

gatorsmile · 2016-08-22T18:32:07Z

retest this please

SparkQA · 2016-08-22T20:42:08Z

Test build #64226 has finished for PR 14625 at commit e2677da.

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

cloud-fan · 2016-08-23T02:21:13Z

hmmm, can we split this PR into multiple PRs? We are not copying tests from HiveCompatibilitySuite directly and it takes time to review each test and its result.

gatorsmile · 2016-08-23T04:01:37Z

Sure, will split it to multiple PRs. Thanks!

gatorsmile added 2 commits August 12, 2016 12:02

first attempt.

6f575a0

add files

9888f8a

rxin reviewed Aug 12, 2016
View reviewed changes

gatorsmile added 3 commits August 12, 2016 23:53

batch 1

239191c

batch 1

aaeb298

revert

5f22316

batch 2

3fe55f1

gatorsmile added 3 commits August 13, 2016 17:34

added meaningful comments

98f7f11

added meaningful comments

a26bdeb

added meaningful comments

cdea1a3

gatorsmile changed the title ~~[SPARK-17045] [SQL] Moving Auto_Joins from HiveCompatibilitySuite to SQLQueryTestSuite~~ [SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite Aug 14, 2016

batch 3

9c67e69

gatorsmile changed the title ~~[SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite [WIP]~~ [SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite Aug 18, 2016

gatorsmile added 2 commits August 17, 2016 17:57

code clean

cf9a233

code clean

b4801e0

gatorsmile added 2 commits August 19, 2016 21:01

Merge remote-tracking branch 'upstream/master' into auto_join

5ecfc4f

update based on the lastest fix.

bf55624

cloud-fan reviewed Aug 21, 2016
View reviewed changes

gatorsmile added 6 commits August 21, 2016 19:51

remove the comments (xyz.q)

42b3c69

change the table name for testData and testData2

1118626

rename src by duplicateRowData

c73134e

rename src1 by nullData

a204f52

rename srcpart by partitionedData

046c3c3

Merge remote-tracking branch 'upstream/master' into auto_join

9c6be8e

gatorsmile added 2 commits August 21, 2016 23:39

fix.

1969ca2

revert

e2677da

gatorsmile closed this Aug 23, 2016

This was referenced Aug 24, 2016

[SPARK-17208] [SQL] Build Outer Join Test Cases in File-based Testing Framework #14782

Closed

[SPARK-17221] [SQL] Build File-based Test Cases for Using Join and Left-Semi Join #14793

Closed

		@@ -0,0 +1,225 @@
		-- join nested table expressions (auto_join0.q)

[SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite #14625

[SPARK-17045] [SQL] Build/move Join-related test cases in SQLQueryTestSuite #14625

Uh oh!

Conversation

gatorsmile commented Aug 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

gatorsmile commented Aug 12, 2016

Uh oh!

rxin Aug 12, 2016

Choose a reason for hiding this comment

Uh oh!

rxin commented Aug 12, 2016

Uh oh!

gatorsmile commented Aug 12, 2016

Uh oh!

rxin commented Aug 12, 2016

Uh oh!

SparkQA commented Aug 12, 2016

Uh oh!

gatorsmile commented Aug 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Aug 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gatorsmile commented Aug 13, 2016

Uh oh!

SparkQA commented Aug 13, 2016

Uh oh!

SparkQA commented Aug 13, 2016

Uh oh!

SparkQA commented Aug 13, 2016

Uh oh!

rxin commented Aug 13, 2016

Uh oh!

gatorsmile commented Aug 13, 2016

Uh oh!

gatorsmile commented Aug 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gatorsmile commented Aug 13, 2016

Uh oh!

rxin commented Aug 13, 2016

Uh oh!

gatorsmile commented Aug 14, 2016

Uh oh!

SparkQA commented Aug 14, 2016

Uh oh!

SparkQA commented Aug 18, 2016

Uh oh!

SparkQA commented Aug 18, 2016

Uh oh!

SparkQA commented Aug 18, 2016

Uh oh!

SparkQA commented Aug 18, 2016

Uh oh!

gatorsmile commented Aug 18, 2016

Uh oh!

SparkQA commented Aug 20, 2016

Uh oh!

cloud-fan Aug 21, 2016

Choose a reason for hiding this comment

Uh oh!

gatorsmile Aug 21, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 22, 2016

Uh oh!

SparkQA commented Aug 22, 2016

Uh oh!

gatorsmile commented Aug 22, 2016

Uh oh!

SparkQA commented Aug 22, 2016

Uh oh!

cloud-fan commented Aug 23, 2016

Uh oh!

gatorsmile commented Aug 23, 2016

gatorsmile commented Aug 12, 2016 •

edited

Loading

gatorsmile commented Aug 12, 2016 •

edited

Loading

cloud-fan commented Aug 13, 2016 •

edited

Loading

gatorsmile commented Aug 13, 2016 •

edited

Loading