From b8a479155c75022c1a4a2aad7bfdc4e5b0bc97bb Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 15 Nov 2016 18:08:40 -0800 Subject: [PATCH 01/10] Added test --- .../scala/org/apache/spark/sql/internal/CatalogImpl.scala | 7 ++++++- .../scala/org/apache/spark/sql/internal/CatalogSuite.scala | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala index d3e323cb12891..4f3bdcdb1f56f 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala @@ -23,6 +23,7 @@ import scala.reflect.runtime.universe.TypeTag import org.apache.spark.annotation.Experimental import org.apache.spark.sql._ import org.apache.spark.sql.catalog.{Catalog, Column, Database, Function, Table} +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{DefinedByConstructorParams, FunctionIdentifier, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder @@ -420,7 +421,11 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog { * @since 2.0.0 */ override def uncacheTable(tableName: String): Unit = { - sparkSession.sharedState.cacheManager.uncacheQuery(query = sparkSession.table(tableName)) + try { + sparkSession.sharedState.cacheManager.uncacheQuery(query = sparkSession.table(tableName)) + } catch { + case _: NoSuchTableException => // do nothing + } } /** diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala index 89ec162c8ed52..b4dbb4e87042d 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala @@ -340,6 +340,11 @@ class CatalogSuite } } + test("uncache table shouldn't throw an exception if table doesn't exist") { + // doesn't throw TableNotFoundException + spark.catalog.uncacheTable("random_table") + } + test("get database") { intercept[AnalysisException](spark.catalog.getDatabase("db10")) withTempDatabase { db => From f62bc2bc9e88b1f136ca70e284dd1ec1d9f66d63 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 15 Nov 2016 18:17:15 -0800 Subject: [PATCH 02/10] make test better --- .../test/scala/org/apache/spark/sql/internal/CatalogSuite.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala index b4dbb4e87042d..52c9a10675671 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala @@ -22,6 +22,7 @@ import org.scalatest.BeforeAndAfterEach import org.apache.spark.SparkFunSuite import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.catalog.{Column, Database, Function, Table} +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{FunctionIdentifier, ScalaReflection, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.expressions.{Expression, ExpressionInfo} @@ -341,6 +342,7 @@ class CatalogSuite } test("uncache table shouldn't throw an exception if table doesn't exist") { + intercept[NoSuchTableException](spark.table("random_table")) // doesn't throw TableNotFoundException spark.catalog.uncacheTable("random_table") } From 8c11e906a14d82fd8e01a245a43b93403670be78 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 15 Nov 2016 18:19:04 -0800 Subject: [PATCH 03/10] made change --- .../org/apache/spark/sql/internal/CatalogSuite.scala | 6 ------ .../org/apache/spark/sql/hive/CachedTableSuite.scala | 12 +++++------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala index 52c9a10675671..b76bd724f2645 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala @@ -341,12 +341,6 @@ class CatalogSuite } } - test("uncache table shouldn't throw an exception if table doesn't exist") { - intercept[NoSuchTableException](spark.table("random_table")) - // doesn't throw TableNotFoundException - spark.catalog.uncacheTable("random_table") - } - test("get database") { intercept[AnalysisException](spark.catalog.getDatabase("db10")) withTempDatabase { db => diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala index fc35304c80ecc..ed3a6eced4034 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala @@ -101,13 +101,11 @@ class CachedTableSuite extends QueryTest with SQLTestUtils with TestHiveSingleto sql("DROP TABLE IF EXISTS nonexistantTable") } - test("correct error on uncache of nonexistant tables") { - intercept[NoSuchTableException] { - spark.catalog.uncacheTable("nonexistantTable") - } - intercept[NoSuchTableException] { - sql("UNCACHE TABLE nonexistantTable") - } + test("uncache of nonexistant tables don't throw exceptions") { + intercept[NoSuchTableException](spark.table("nonexistantTable")) + // doesn't throw NoSuchTableException + spark.catalog.uncacheTable("nonexistantTable") + sql("UNCACHE TABLE nonexistantTable") } test("no error on uncache of non-cached table") { From 27acd46e3f04b1ab9ea7b95e5e62a03f90e58faf Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Wed, 16 Nov 2016 08:37:18 -0800 Subject: [PATCH 04/10] Update CatalogImpl.scala --- .../main/scala/org/apache/spark/sql/internal/CatalogImpl.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala index 4f3bdcdb1f56f..70f5d02c084d4 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala @@ -23,8 +23,8 @@ import scala.reflect.runtime.universe.TypeTag import org.apache.spark.annotation.Experimental import org.apache.spark.sql._ import org.apache.spark.sql.catalog.{Catalog, Column, Database, Function, Table} -import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{DefinedByConstructorParams, FunctionIdentifier, TableIdentifier} +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder import org.apache.spark.sql.catalyst.plans.logical.LocalRelation From 98b22d53cb2b7c88cb8503e6b879c7ce0e62c51f Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Wed, 16 Nov 2016 08:52:53 -0800 Subject: [PATCH 05/10] Update CatalogSuite.scala --- .../test/scala/org/apache/spark/sql/internal/CatalogSuite.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala index b76bd724f2645..89ec162c8ed52 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala @@ -22,7 +22,6 @@ import org.scalatest.BeforeAndAfterEach import org.apache.spark.SparkFunSuite import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.catalog.{Column, Database, Function, Table} -import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{FunctionIdentifier, ScalaReflection, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.expressions.{Expression, ExpressionInfo} From 15f3e59f78b1542af6970feb22620a9d75697241 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Wed, 16 Nov 2016 11:13:50 -0800 Subject: [PATCH 06/10] Update tests.py --- python/pyspark/sql/tests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/python/pyspark/sql/tests.py b/python/pyspark/sql/tests.py index 3d46b852c52e1..450f9a2fff470 100644 --- a/python/pyspark/sql/tests.py +++ b/python/pyspark/sql/tests.py @@ -1716,10 +1716,6 @@ def test_cache(self): AnalysisException, "does_not_exist", lambda: spark.catalog.cacheTable("does_not_exist")) - self.assertRaisesRegexp( - AnalysisException, - "does_not_exist", - lambda: spark.catalog.uncacheTable("does_not_exist")) def test_read_text_file_list(self): df = self.spark.read.text(['python/test_support/sql/text-test.txt', From 0c85fc017e751ce7082e8f2c1632a593ff808fe2 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 22 Nov 2016 13:10:33 -0500 Subject: [PATCH 07/10] added if exists clause to uncache table --- .../org/apache/spark/sql/catalyst/parser/SqlBase.g4 | 2 +- .../apache/spark/sql/execution/SparkSqlParser.scala | 2 +- .../apache/spark/sql/execution/command/cache.scala | 13 +++++++++++-- .../org/apache/spark/sql/internal/CatalogImpl.scala | 7 +------ .../apache/spark/sql/internal/CatalogSuite.scala | 1 - .../apache/spark/sql/hive/CachedTableSuite.scala | 13 +++++++++---- 6 files changed, 23 insertions(+), 15 deletions(-) diff --git a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 index fcca11c69f0a3..bd05855f0a19d 100644 --- a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 +++ b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 @@ -142,7 +142,7 @@ statement | REFRESH TABLE tableIdentifier #refreshTable | REFRESH .*? #refreshResource | CACHE LAZY? TABLE tableIdentifier (AS? query)? #cacheTable - | UNCACHE TABLE tableIdentifier #uncacheTable + | UNCACHE TABLE (IF EXISTS)? tableIdentifier #uncacheTable | CLEAR CACHE #clearCache | LOAD DATA LOCAL? INPATH path=STRING OVERWRITE? INTO TABLE tableIdentifier partitionSpec? #loadData diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index 112d812cb6c76..df509a56792e6 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -233,7 +233,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { * Create an [[UncacheTableCommand]] logical plan. */ override def visitUncacheTable(ctx: UncacheTableContext): LogicalPlan = withOrigin(ctx) { - UncacheTableCommand(visitTableIdentifier(ctx.tableIdentifier)) + UncacheTableCommand(visitTableIdentifier(ctx.tableIdentifier), ctx.EXISTS != null) } /** diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala index c31f4dc9aba4b..cedc649871f3b 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala @@ -19,6 +19,7 @@ package org.apache.spark.sql.execution.command import org.apache.spark.sql.{Dataset, Row, SparkSession} import org.apache.spark.sql.catalyst.TableIdentifier +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.plans.QueryPlan import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan @@ -49,10 +50,18 @@ case class CacheTableCommand( } -case class UncacheTableCommand(tableIdent: TableIdentifier) extends RunnableCommand { +case class UncacheTableCommand( + tableIdent: TableIdentifier, + ifExists: Boolean) extends RunnableCommand { override def run(sparkSession: SparkSession): Seq[Row] = { - sparkSession.catalog.uncacheTable(tableIdent.quotedString) + val tableId = tableIdent.quotedString + try { + sparkSession.catalog.uncacheTable(tableId) + } catch { + case _: NoSuchTableException if ifExists => // don't throw + logInfo(s"Asked to uncache table $tableId which doesn't exist.") + } Seq.empty[Row] } } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala index 4f3bdcdb1f56f..d3e323cb12891 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala @@ -23,7 +23,6 @@ import scala.reflect.runtime.universe.TypeTag import org.apache.spark.annotation.Experimental import org.apache.spark.sql._ import org.apache.spark.sql.catalog.{Catalog, Column, Database, Function, Table} -import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{DefinedByConstructorParams, FunctionIdentifier, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder @@ -421,11 +420,7 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog { * @since 2.0.0 */ override def uncacheTable(tableName: String): Unit = { - try { - sparkSession.sharedState.cacheManager.uncacheQuery(query = sparkSession.table(tableName)) - } catch { - case _: NoSuchTableException => // do nothing - } + sparkSession.sharedState.cacheManager.uncacheQuery(query = sparkSession.table(tableName)) } /** diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala index b76bd724f2645..89ec162c8ed52 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala @@ -22,7 +22,6 @@ import org.scalatest.BeforeAndAfterEach import org.apache.spark.SparkFunSuite import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.catalog.{Column, Database, Function, Table} -import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.{FunctionIdentifier, ScalaReflection, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.expressions.{Expression, ExpressionInfo} diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala index ed3a6eced4034..3871b3d785882 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala @@ -101,11 +101,16 @@ class CachedTableSuite extends QueryTest with SQLTestUtils with TestHiveSingleto sql("DROP TABLE IF EXISTS nonexistantTable") } - test("uncache of nonexistant tables don't throw exceptions") { + test("uncache of nonexistant tables") { + // make sure table doesn't exist intercept[NoSuchTableException](spark.table("nonexistantTable")) - // doesn't throw NoSuchTableException - spark.catalog.uncacheTable("nonexistantTable") - sql("UNCACHE TABLE nonexistantTable") + intercept[NoSuchTableException] { + spark.catalog.uncacheTable("nonexistantTable") + } + intercept[NoSuchTableException] { + sql("UNCACHE TABLE nonexistantTable") + } + sql("UNCACHE TABLE IF EXISTS nonexistantTable") } test("no error on uncache of non-cached table") { From 266b90217c4fdc79054020df41b6d679b283f252 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 22 Nov 2016 13:12:56 -0500 Subject: [PATCH 08/10] re-add pyspark test --- python/pyspark/sql/tests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/pyspark/sql/tests.py b/python/pyspark/sql/tests.py index 450f9a2fff470..3d46b852c52e1 100644 --- a/python/pyspark/sql/tests.py +++ b/python/pyspark/sql/tests.py @@ -1716,6 +1716,10 @@ def test_cache(self): AnalysisException, "does_not_exist", lambda: spark.catalog.cacheTable("does_not_exist")) + self.assertRaisesRegexp( + AnalysisException, + "does_not_exist", + lambda: spark.catalog.uncacheTable("does_not_exist")) def test_read_text_file_list(self): df = self.spark.read.text(['python/test_support/sql/text-test.txt', From 4d62ce4ff804a4b46e619f91f3d8ab1ba405d6df Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 22 Nov 2016 13:17:36 -0500 Subject: [PATCH 09/10] remove unnecessary import --- .../main/scala/org/apache/spark/sql/internal/CatalogImpl.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala index 68d3a80129dab..d3e323cb12891 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala @@ -24,7 +24,6 @@ import org.apache.spark.annotation.Experimental import org.apache.spark.sql._ import org.apache.spark.sql.catalog.{Catalog, Column, Database, Function, Table} import org.apache.spark.sql.catalyst.{DefinedByConstructorParams, FunctionIdentifier, TableIdentifier} -import org.apache.spark.sql.catalyst.analysis.NoSuchTableException import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder import org.apache.spark.sql.catalyst.plans.logical.LocalRelation From 5432a83230f86f426a6ac4070b8eda103241ae19 Mon Sep 17 00:00:00 2001 From: Burak Yavuz Date: Tue, 22 Nov 2016 14:16:17 -0500 Subject: [PATCH 10/10] don't log --- .../scala/org/apache/spark/sql/execution/command/cache.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala index cedc649871f3b..336f14dd97aea 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala @@ -60,7 +60,6 @@ case class UncacheTableCommand( sparkSession.catalog.uncacheTable(tableId) } catch { case _: NoSuchTableException if ifExists => // don't throw - logInfo(s"Asked to uncache table $tableId which doesn't exist.") } Seq.empty[Row] }