ODIn/spark-instrumented-optimizer
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[SPARK-27772][SQL][TEST] Refactor SQLTestUtils to use tryWithSafeFinally

Browse source 
## What changes were proposed in this pull request?

The current `SQLTestUtils` created many `withXXX` utility functions to clean up tables/views/caches created for testing purpose. Java's `try-with-resources` statement does something similar, but it does not mask exception throwing in the try block with any exception caught in the 'close()' statement. Exception caught in the 'close()' statement would add as a suppressed exception instead.

This PR standardizes those 'withXXX' function to use`Utils.tryWithSafeFinally` function, which does something similar to Java's try-with-resources statement. The purpose of this proposal is to help developers to identify what actually breaks their tests.

## How was this patch tested?
Existing testcases.

Closes #24747 from William1104/feature/SPARK-27772-2.

Lead-authored-by: williamwong <william1104@gmail.com>
Co-authored-by: William Wong <william1104@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
This commit is contained in:
williamwong 2019-06-04 09:26:24 -05:00 committed by Sean Owen
parent a38d605d0d
commit d5715a9b23
1 changed files with 20 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala
View file

@ -255,11 +255,13 @@ private[sql] trait SQLTestUtilsBase
* Drops temporary view `viewNames` after calling `f`. * Drops temporary view `viewNames` after calling `f`.
*/ */
protected def withTempView(viewNames: String*)(f: => Unit): Unit = { protected def withTempView(viewNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f) {
// If the test failed part way, we don't want to mask the failure by failing to remove viewNames.foreach { viewName =>
// temp views that never got created. try spark.catalog.dropTempView(viewName) catch {
try viewNames.foreach(spark.catalog.dropTempView) catch { // If the test failed part way, we don't want to mask the failure by failing to remove
case _: NoSuchTableException => // temp views that never got created.
case _: NoSuchTableException =>
}
} }
} }
} }
@ -268,11 +270,13 @@ private[sql] trait SQLTestUtilsBase
* Drops global temporary view `viewNames` after calling `f`. * Drops global temporary view `viewNames` after calling `f`.
*/ */
protected def withGlobalTempView(viewNames: String*)(f: => Unit): Unit = { protected def withGlobalTempView(viewNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f) {
// If the test failed part way, we don't want to mask the failure by failing to remove viewNames.foreach { viewName =>
// global temp views that never got created. try spark.catalog.dropGlobalTempView(viewName) catch {
try viewNames.foreach(spark.catalog.dropGlobalTempView) catch { // If the test failed part way, we don't want to mask the failure by failing to remove
case _: NoSuchTableException => // global temp views that never got created.
case _: NoSuchTableException =>
}
} }
} }
} }
@ -281,7 +285,7 @@ private[sql] trait SQLTestUtilsBase
* Drops table `tableName` after calling `f`. * Drops table `tableName` after calling `f`.
*/ */
protected def withTable(tableNames: String*)(f: => Unit): Unit = { protected def withTable(tableNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f) {
tableNames.foreach { name => tableNames.foreach { name =>
spark.sql(s"DROP TABLE IF EXISTS $name") spark.sql(s"DROP TABLE IF EXISTS $name")
} }
@ -292,18 +296,18 @@ private[sql] trait SQLTestUtilsBase
* Drops view `viewName` after calling `f`. * Drops view `viewName` after calling `f`.
*/ */
protected def withView(viewNames: String*)(f: => Unit): Unit = { protected def withView(viewNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f)(
viewNames.foreach { name => viewNames.foreach { name =>
spark.sql(s"DROP VIEW IF EXISTS $name") spark.sql(s"DROP VIEW IF EXISTS $name")
} }
} )
} }
/** /**
* Drops cache `cacheName` after calling `f`. * Drops cache `cacheName` after calling `f`.
*/ */
protected def withCache(cacheNames: String*)(f: => Unit): Unit = { protected def withCache(cacheNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f) {
cacheNames.foreach { cacheName => cacheNames.foreach { cacheName =>
try uncacheTable(cacheName) catch { try uncacheTable(cacheName) catch {
case _: AnalysisException => case _: AnalysisException =>
@ -350,7 +354,7 @@ private[sql] trait SQLTestUtilsBase
* Drops database `dbName` after calling `f`. * Drops database `dbName` after calling `f`.
*/ */
protected def withDatabase(dbNames: String*)(f: => Unit): Unit = { protected def withDatabase(dbNames: String*)(f: => Unit): Unit = {
try f finally { Utils.tryWithSafeFinally(f) {
dbNames.foreach { name => dbNames.foreach { name =>
spark.sql(s"DROP DATABASE IF EXISTS $name CASCADE") spark.sql(s"DROP DATABASE IF EXISTS $name CASCADE")
} }
@ -379,7 +383,7 @@ private[sql] trait SQLTestUtilsBase
*/ */
protected def activateDatabase(db: String)(f: => Unit): Unit = { protected def activateDatabase(db: String)(f: => Unit): Unit = {
spark.sessionState.catalog.setCurrentDatabase(db) spark.sessionState.catalog.setCurrentDatabase(db)
try f finally spark.sessionState.catalog.setCurrentDatabase("default") Utils.tryWithSafeFinally(f)(spark.sessionState.catalog.setCurrentDatabase("default"))
} }
/** /**