[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/19888 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r155022526 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -203,14 +203,20 @@ case class DropTableCommand( case _ => } } -try { - sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) -} catch { - case _: NoSuchTableException if ifExists => - case NonFatal(e) => log.warn(e.toString, e) + +if (catalog.isTemporaryTable(tableName) || catalog.tableExists(tableName)) { + try { + sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) + } catch { +case NonFatal(e) => log.warn(e.toString, e) + } + catalog.refreshTable(tableName) + catalog.dropTable(tableName, ifExists, purge) +} else if (ifExists) { + // no-op --- End diff -- ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154992063 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -203,14 +203,16 @@ case class DropTableCommand( case _ => } } -try { - sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) -} catch { - case _: NoSuchTableException if ifExists => - case NonFatal(e) => log.warn(e.toString, e) + +if (!ifExists || catalog.tableExists(tableName)) { --- End diff -- I'll update soon. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154883352 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -203,14 +203,16 @@ case class DropTableCommand( case _ => } } -try { - sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) -} catch { - case _: NoSuchTableException if ifExists => - case NonFatal(e) => log.warn(e.toString, e) + +if (!ifExists || catalog.tableExists(tableName)) { --- End diff -- We can also update the UNCACHE TABLE, cc @gatorsmile --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154883267 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -203,14 +203,16 @@ case class DropTableCommand( case _ => } } -try { - sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) -} catch { - case _: NoSuchTableException if ifExists => - case NonFatal(e) => log.warn(e.toString, e) + +if (!ifExists || catalog.tableExists(tableName)) { --- End diff -- I feel it's more readable to write ``` if (catalog.tableExists(tableName)) { ... do the drop } else if (ifExists) { // do nothing } else { // log.warn("The table to drop does not exist: " +tableName) } ``` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154857148 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- Yep. It exists in master, too. I'll update like the example by @cloud-fan . --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154855703 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- I can reproduce it in master now. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154854071 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- @dongjoon-hyun can you verify it? thanks! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154854042 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- hmmm, it's fixed in master? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154853724 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- I can see it in 2.2.1. It is a warning message. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154852342 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- it's in the log, which means it's not a serious bug. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154852162 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- ``` scala> spark.version res2: String = 2.3.0-SNAPSHOT scala> sql("DROP TABLE IF EXISTS t") res3: org.apache.spark.sql.DataFrame = [] scala> sql("DROP TABLE IF EXISTS t") res4: org.apache.spark.sql.DataFrame = [] ``` Unable to reproduce it. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/19888#discussion_r154851944 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala --- @@ -206,7 +206,8 @@ case class DropTableCommand( try { sparkSession.sharedState.cacheManager.uncacheQuery(sparkSession.table(tableName)) } catch { - case _: NoSuchTableException if ifExists => + case ae: AnalysisException +if ifExists && ae.cause.nonEmpty && ae.getCause.isInstanceOf[NoSuchTableException] => --- End diff -- can we follow https://github.com/apache/spark/pull/19713/files#diff-bc55b5f76add105ec32ae4107075b278R57 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19888: [SPARK-22686][SQL] DROP TABLE IF EXISTS should no...
GitHub user dongjoon-hyun opened a pull request: https://github.com/apache/spark/pull/19888 [SPARK-22686][SQL] DROP TABLE IF EXISTS should not throw AnalysisException ## What changes were proposed in this pull request? During [SPARK-22488](https://github.com/apache/spark/pull/19713) to fix view resolution issue, there occurs a regression at `2.2.1` and `master` branch like the following. This PR fixes that. ```scala scala> spark.version res2: String = 2.2.1 scala> sql("DROP TABLE IF EXISTS t").show 17/12/04 21:01:06 WARN DropTableCommand: org.apache.spark.sql.AnalysisException: Table or view not found: t; org.apache.spark.sql.AnalysisException: Table or view not found: t; ``` ## How was this patch tested? Manual. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongjoon-hyun/spark SPARK-22686 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/19888.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #19888 commit 2b113a2871dc732d74b0564017bed15e53b297ab Author: Dongjoon Hyun Date: 2017-12-05T05:01:53Z [SPARK-22686][SQL] DROP TABLE IF EXISTS should not throw AnalysisException --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org