-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-17008][SPARK-17009][SQL] Normalization and isolation in SQLQueryTestSuite. #14590
Conversation
… isolation in SQLQueryTestSuite.
This reverts commit 1c9e502.
@cloud-fan an update here. There is another one I want to do to improve exception handling for negative cases that I will do in a separate pull request when I port some other tests. |
Test build #63572 has finished for PR 14590 at commit
|
@@ -0,0 +1,10 @@ | |||
-- Automatically generated by org.apache.spark.sql.SQLQueryTestSuite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to remove the package name so we don't need to change all the generated files when we move this class.
The failed Python test is unrelated. I'm going to merge this in master. Thanks. |
@@ -126,14 +129,18 @@ class SQLQueryTestSuite extends QueryTest with SharedSQLContext { | |||
cleaned.split("(?<=[^\\\\]);").map(_.trim).filter(_ != "").toSeq | |||
} | |||
|
|||
// Create a local SparkSession to have stronger isolation between different test cases. | |||
// This does not isolate catalog changes. | |||
val localSparkSession = spark.newSession() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expensive? I do remember other tests share one spark session for performance reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SparkSession should be fine. SparkContext is the expensive one.
Test build #3216 has finished for PR 14590 at commit
|
…ryTestSuite. ## What changes were proposed in this pull request? This patch enhances SQLQueryTestSuite in two ways: 1. SPARK-17009: Use a new SparkSession for each test case to provide stronger isolation (e.g. config changes in one test case does not impact another). That said, we do not currently isolate catalog changes. 2. SPARK-17008: Normalize query output using sorting, inspired by HiveComparisonTest. I also ported a few new test cases over from SQLQuerySuite. ## How was this patch tested? This is a test harness update. Author: petermaxlee <petermaxlee@gmail.com> Closes #14590 from petermaxlee/SPARK-17008. (cherry picked from commit 425c7c2) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
backport to 2.0! |
What changes were proposed in this pull request?
This patch enhances SQLQueryTestSuite in two ways:
I also ported a few new test cases over from SQLQuerySuite.
How was this patch tested?
This is a test harness update.