[SPARK-10176][SQL] Show partially analyzed plans when checkAnswer fails to analyze #8584

cloud-fan · 2015-09-03T14:25:48Z

This PR takes over #8389.

This PR improves checkAnswer to print the partially analyzed plan in addition to the user friendly error message, in order to aid debugging failing tests.

In doing so, I ran into a conflict with the various ways that we bring a SQLContext into the tests. Depending on the trait we refer to the current context as sqlContext, _sqlContext, ctx or hiveContext with access modifiers public, protected and private depending on the defining class.

I propose we refactor as follows:

All tests should only refer to a protected sqlContext when testing general features, and protected hiveContext when it is a method that only exists on a HiveContext.
All tests should only import testImplicits._ (i.e., don't import TestHive.implicits._)

cloud-fan · 2015-09-03T14:44:54Z

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

I removed the ctx alias for sqlContext, and changed a lot of lines of code which is trival actually. It's not a big deal to keep an alias, so if you think it's unnecessary, I'll roll back it.

+1. Since we often deal with both spark and sql contexts, I'd like to uniformly use sqlContext throughout the codebase.

SparkQA · 2015-09-03T14:45:38Z

Test build #41970 has finished for PR 8584 at commit 41a5f90.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class QueryTest extends PlanTest
- class PlannerSuite extends SharedSQLContext
- class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter
- implicit class StringToColumn(val sc: StringContext)
- trait TestHiveSingleton
- class CachedTableSuite extends QueryTest with TestHiveSingleton
- class ErrorPositionSuite extends QueryTest with TestHiveSingleton with BeforeAndAfter
- class HiveDataFrameAnalyticsSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- class HiveDataFrameJoinSuite extends QueryTest with TestHiveSingleton
- class HiveDataFrameWindowSuite extends QueryTest with TestHiveSingleton
- class HiveMetastoreCatalogSuite extends SparkFunSuite with TestHiveSingleton
- class HiveParquetSuite extends QueryTest with ParquetTest with TestHiveSingleton
- class InsertIntoHiveTableSuite extends QueryTest with TestHiveSingleton with BeforeAndAfter
- class ListTablesSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class MultiDatabaseSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class ParquetHiveCompatibilitySuite extends ParquetCompatibilityTest with TestHiveSingleton
- class QueryPartitionSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class StatisticsSuite extends QueryTest with TestHiveSingleton
- class UDFSuite extends QueryTest with TestHiveSingleton
- abstract class AggregationQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class HiveExplainSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class HiveOperatorQueryableSuite extends QueryTest with TestHiveSingleton
- class HivePlanTest extends QueryTest with TestHiveSingleton
- class HiveUDFSuite extends QueryTest with TestHiveSingleton
- class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class ScriptTransformationSuite extends SparkPlanTest with TestHiveSingleton
- class OrcPartitionDiscoverySuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- abstract class OrcSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- abstract class ParquetPartitioningTest extends QueryTest with SQLTestUtils with TestHiveSingleton
- class CommitFailureTestRelationSuite extends SQLTestUtils with TestHiveSingleton
- abstract class HadoopFsRelationTest extends QueryTest with SQLTestUtils with TestHiveSingleton

cloud-fan · 2015-09-03T14:51:37Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala

I can't access SQLTestUtils in this file, not sure why.

yeah weird, I tried it locally too and I can't even import it, we can just leave it then.

SQLTestUtils is in test scope. TestHive is in main.

OK we can just leave it then. (why is TestHive in main??)

It is used by the hive console (build/sbt -Phive hive/console).

cloud-fan · 2015-09-03T15:06:36Z

retest this please.

cloud-fan · 2015-09-03T15:10:33Z

cc @andrewor14

SparkQA · 2015-09-03T15:27:26Z

Test build #41973 has finished for PR 8584 at commit a9ae3f5.

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

yhuai · 2015-09-03T16:00:00Z

There is compilation error in InMemoryColumnarQuerySuite. Also, why this PR has significantly more changes than the original one?

andrewor14 · 2015-09-03T16:01:36Z

I think it's cause it renames ctx to sqlContext, which I think @marmbrus agrees is a good idea IIRC.

SparkQA · 2015-09-03T16:11:08Z

Test build #41975 has finished for PR 8584 at commit 0faa5ba.

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

andrewor14 · 2015-09-03T21:01:49Z

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

minor: these could just be sparkContext I think. You can do a grep to find these.

andrewor14 · 2015-09-03T21:17:00Z

Looks great. There are many places where we still do sqlContext.sparkContext but that's no longer necessary. You can to a search and replace to clean that up (not all occurrences are relevant, however). Looks like it's still failing the style test. Can you fix it?

SparkQA · 2015-09-04T04:13:18Z

Test build #41988 has finished for PR 8584 at commit 6db29b5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class QueryTest extends PlanTest
- class PlannerSuite extends SharedSQLContext
- class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter
- implicit class StringToColumn(val sc: StringContext)
- trait TestHiveSingleton
- class CachedTableSuite extends QueryTest with TestHiveSingleton
- class ErrorPositionSuite extends QueryTest with TestHiveSingleton with BeforeAndAfter
- class HiveDataFrameAnalyticsSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- class HiveDataFrameJoinSuite extends QueryTest with TestHiveSingleton
- class HiveDataFrameWindowSuite extends QueryTest with TestHiveSingleton
- class HiveMetastoreCatalogSuite extends SparkFunSuite with TestHiveSingleton
- class HiveParquetSuite extends QueryTest with ParquetTest with TestHiveSingleton
- class InsertIntoHiveTableSuite extends QueryTest with TestHiveSingleton with BeforeAndAfter
- class ListTablesSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class MultiDatabaseSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class ParquetHiveCompatibilitySuite extends ParquetCompatibilityTest with TestHiveSingleton
- class QueryPartitionSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class StatisticsSuite extends QueryTest with TestHiveSingleton
- class UDFSuite extends QueryTest with TestHiveSingleton
- abstract class AggregationQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class HiveExplainSuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class HiveOperatorQueryableSuite extends QueryTest with TestHiveSingleton
- class HivePlanTest extends QueryTest with TestHiveSingleton
- class HiveUDFSuite extends QueryTest with TestHiveSingleton
- class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton
- class ScriptTransformationSuite extends SparkPlanTest with TestHiveSingleton
- class OrcPartitionDiscoverySuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- abstract class OrcSuite extends QueryTest with TestHiveSingleton with BeforeAndAfterAll
- abstract class ParquetPartitioningTest extends QueryTest with SQLTestUtils with TestHiveSingleton
- class CommitFailureTestRelationSuite extends SQLTestUtils with TestHiveSingleton
- abstract class HadoopFsRelationTest extends QueryTest with SQLTestUtils with TestHiveSingleton

andrewor14 · 2015-09-04T17:48:11Z

OK LGTM I'm merging this into master thanks @marmbrus @cloud-fan

yhuai · 2015-09-04T22:11:27Z

@andrewor14 have you merged it?

Jenkins master builders are currently broken by a merge conflict between PR #8584 and PR #8155. Author: Cheng Lian <[email protected]> Closes #8614 from liancheng/hotfix/fix-pr-8155-8584-conflict.

cloud-fan force-pushed the cleanupTests branch 3 times, most recently from 61708d8 to 41a5f90 Compare September 3, 2015 14:34

cloud-fan reviewed Sep 3, 2015
View reviewed changes

cloud-fan force-pushed the cleanupTests branch from 41a5f90 to a9ae3f5 Compare September 3, 2015 15:04

cloud-fan force-pushed the cleanupTests branch from a9ae3f5 to 0faa5ba Compare September 3, 2015 15:59

andrewor14 reviewed Sep 3, 2015
View reviewed changes

cloud-fan added 2 commits September 4, 2015 08:54

clean up sql tests

0183419

use sparkContext directly instead of sqlContext.sparkContext

6db29b5

cloud-fan force-pushed the cleanupTests branch from 0faa5ba to 6db29b5 Compare September 4, 2015 01:58

marmbrus mentioned this pull request Sep 4, 2015

[SPARK-10176][SQL] Show partially analyzed plans when checkAnswer fails to analyze #8389

Closed

asfgit closed this in c3c0e43 Sep 4, 2015

cloud-fan deleted the cleanupTests branch September 5, 2015 03:21

cloud-fan mentioned this pull request Sep 5, 2015

[SPARK-9925] [SQL] [TESTS] Set SQLConf.SHUFFLE_PARTITIONS.key correctly for tests #8602

Closed

liancheng mentioned this pull request Sep 5, 2015

[HOTFIX] [SQL] Fixes compilation error #8614

Closed

liancheng mentioned this pull request Sep 11, 2015

[HotFix][SQL] Fix SQLConfSuite #8612

Closed

[SPARK-10176][SQL] Show partially analyzed plans when checkAnswer fails to analyze #8584

[SPARK-10176][SQL] Show partially analyzed plans when checkAnswer fails to analyze #8584

Uh oh!

Conversation

cloud-fan commented Sep 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Sep 3, 2015

Uh oh!

cloud-fan commented Sep 3, 2015

Uh oh!

SparkQA commented Sep 3, 2015

Uh oh!

yhuai commented Sep 3, 2015

Uh oh!

andrewor14 commented Sep 3, 2015

Uh oh!

SparkQA commented Sep 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Sep 3, 2015

Uh oh!

SparkQA commented Sep 4, 2015

Uh oh!

andrewor14 commented Sep 4, 2015

Uh oh!

yhuai commented Sep 4, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants