[ 
https://issues.apache.org/jira/browse/SPARK-27772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16850264#comment-16850264
 ] 

Hyukjin Kwon commented on SPARK-27772:
--------------------------------------

k. please clarify the problem with before/after message to show that it's 
useful for dev.

> SQLTestUtils Refactoring
> ------------------------
>
>                 Key: SPARK-27772
>                 URL: https://issues.apache.org/jira/browse/SPARK-27772
>             Project: Spark
>          Issue Type: Improvement
>          Components: SQL
>    Affects Versions: 3.0.0
>            Reporter: William Wong
>            Priority: Minor
>
> The current `SQLTestUtils` created many `withXXX` utility functions to clean 
> up tables/views/caches created for testing purpose. Some of those `withXXX` 
> functions ignore certain exceptions, like `NoSuchTableException` in the clean 
> up block (ie, the finally block). 
>  
> {code:java}
> /**
>  * Drops temporary view `viewNames` after calling `f`.
>  */
> protected def withTempView(viewNames: String*)(f: => Unit): Unit = {
>   try f finally {
>     // If the test failed part way, we don't want to mask the failure by 
> failing to remove
>     // temp views that never got created.
>     try viewNames.foreach(spark.catalog.dropTempView) catch {
>       case _: NoSuchTableException =>
>     }
>   }
> }
> {code}
> Maybe it is not the best approach. Because it is hard to anticipate what 
> exception should or should not be ignored.  
>  
> We may hit similar scenario with Java's `try-with-resources` statement. Java 
> does not mask exception throws 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. IMHO, it is a better approach.  
>  
> Therefore, I proposed to standardise those 'withXXX' function with following 
> `withFinallyBlock` function, which does something similar to Java's 
> try-with-resources statement. 
> {code:java}
> /**
> * Drops temporary view `viewNames` after calling `f`.
> */
> protected def withTempView(viewNames: String*)(f: => Unit): Unit = {
>   withFinallyBlock(f)(viewNames.foreach(spark.catalog.dropTempView))
> }
> /**
>  * Executes the given tryBlock and then the given finallyBlock no matter 
> whether tryBlock throws
>  * an exception. If both tryBlock and finallyBlock throw exceptions, the 
> exception thrown
>  * from the finallyBlock with be added to the exception thrown from tryBlock 
> as a
>  * suppress exception. It helps to avoid masking the exception from tryBlock 
> with exception
>  * from finallyBlock
>  */
> private def withFinallyBlock(tryBlock: => Unit)(finallyBlock: => Unit): Unit 
> = {
>   var fromTryBlock : Throwable = null
>   try tryBlock catch {
>     case cause : Throwable =>
>       fromTryBlock = cause
>       throw cause
>   } finally {
>     if (fromTryBlock != null) {
>       try finallyBlock catch {
>         case fromFinallyBlock : Throwable =>
>           fromTryBlock.addSuppressed(fromFinallyBlock)
>           throw fromTryBlock
>       }
>     } else {
>       finallyBlock
>     }
>   }
> }
> {code}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@spark.apache.org
For additional commands, e-mail: issues-h...@spark.apache.org

Reply via email to