William1104 commented on a change in pull request #24221: [SPARK-27248][SQL]
`refreshTable` should recreate cache with same cache name and storage level
URL: https://github.com/apache/spark/pull/24221#discussion_r285373159
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala
##########
@@ -248,57 +244,91 @@ private[sql] trait SQLTestUtilsBase
!spark.sessionState.catalog.functionExists(FunctionIdentifier(functionName)),
s"Function $functionName should have been dropped. But, it still
exists.")
}
- }
+ )
}
/**
* 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 =>
- }
- }
+ tryWithFinallyBlock(f)(viewNames.foreach(spark.catalog.dropTempView))
Review comment:
They are not equivalent. I thought if an unexpected exception happened
before the finally block, that exception should be thrown instead. I hope the
function 'tryWithFinallyBlock' does the right thing.
Understood the concern and the last commit is just reverted. I will submit
another PR for the function 'tryWithFinallyBlock'. I hope it make senses.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
