-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[MINOR] [SQL] [TEST] Test case cleanups for recent PRs #20341
Conversation
sql(s"DROP TEMPORARY FUNCTION IF EXISTS hive_max") | ||
try { | ||
sql(s"DROP TEMPORARY FUNCTION IF EXISTS mock") | ||
sql(s"DROP TEMPORARY FUNCTION IF EXISTS hive_max") |
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.
Actually, these drop functions are unnecessary. However, it sounds also fine to keep them because we should clean up the local objects after usage.
@@ -492,8 +492,7 @@ private[hive] class TestHiveSparkSession( | |||
protected val originalUDFs: JavaSet[String] = FunctionRegistry.getFunctionNames | |||
|
|||
/** | |||
* Resets the test instance by deleting any tables that have been created. | |||
* TODO: also clear out UDFs, views, etc. |
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.
These TODOs have been addressed.
val dfOne = df.select(lit(1).as("a")) | ||
val dfTwo = spark.range(10).select(lit(2).as("b")) | ||
dfOne.join(dfTwo, $"a" === $"b", "left").queryExecution.optimizedPlan | ||
} |
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.
cc @mgaido91
cc @cloud-fan |
LGTM |
Test build #86440 has finished for PR 20341 at commit
|
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.
+1, LGTM.
BTW, could you add [TEST]
at the title, too?
## What changes were proposed in this pull request? Revert the unneeded test case changes we made in SPARK-23000 Also fixes the test suites that do not call `super.afterAll()` in the local `afterAll`. The `afterAll()` of `TestHiveSingleton` actually reset the environments. ## How was this patch tested? N/A Author: gatorsmile <gatorsmile@gmail.com> Closes #20341 from gatorsmile/testRelated. (cherry picked from commit 896e45a) Signed-off-by: gatorsmile <gatorsmile@gmail.com>
What changes were proposed in this pull request?
Revert the unneeded test case changes we made in SPARK-23000
Also fixes the test suites that do not call
super.afterAll()
in the localafterAll
. TheafterAll()
ofTestHiveSingleton
actually reset the environments.How was this patch tested?
N/A