From 50e8ec77caab263ae5c375f943a61f9278d3dc36 Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Fri, 24 Dec 2021 09:41:17 +0700 Subject: [PATCH 1/7] follow-up: unify drop namespace error --- .../spark/sql/catalyst/catalog/InMemoryCatalog.scala | 4 ++-- .../spark/sql/errors/QueryCompilationErrors.scala | 10 ++++++++-- .../spark/sql/errors/QueryExecutionErrors.scala | 6 ------ .../execution/datasources/v2/DropNamespaceExec.scala | 6 +++--- .../execution/command/DropNamespaceSuiteBase.scala | 7 +++++-- .../sql/execution/command/v1/DropNamespaceSuite.scala | 7 +------ .../sql/execution/command/v2/DropNamespaceSuite.scala | 11 +---------- .../apache/spark/sql/hive/HiveExternalCatalog.scala | 10 +++++++++- 8 files changed, 29 insertions(+), 32 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala index c10e0bbfde213..9cc727e297cef 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala @@ -140,10 +140,10 @@ class InMemoryCatalog( if (!cascade) { // If cascade is false, make sure the database is empty. if (catalog(db).tables.nonEmpty) { - throw QueryCompilationErrors.databaseNotEmptyError(db, "tables") + throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db, "tables") } if (catalog(db).functions.nonEmpty) { - throw QueryCompilationErrors.databaseNotEmptyError(db, "functions") + throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db, "functions") } } // Remove the database. diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala index 79de8b6a1d956..317b629bd1d77 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala @@ -540,8 +540,14 @@ object QueryCompilationErrors { s"rename temporary view from '$oldName' to '$newName': destination view already exists") } - def databaseNotEmptyError(db: String, details: String): Throwable = { - new AnalysisException(s"Database $db is not empty. One or more $details exist.") + def cannotDropNonemptyDatabaseError(db: String, details: String): Throwable = { + new AnalysisException(s"Cannot drop a non-empty database: $db. One or more $details exist.") + } + + def cannotDropNonemptyNamespaceError(namespace: Seq[String]): Throwable = { + new AnalysisException( + s"Cannot drop a non-empty namespace: ${namespace.quoted}. " + + "Use CASCADE option to drop a non-empty namespace.") } def invalidNameForTableOrDatabaseError(name: String): Throwable = { diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala index 6f0ed23e10228..31d17fce09b20 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala @@ -610,12 +610,6 @@ object QueryExecutionErrors { "Schema of v1 relation: " + v1Schema) } - def cannotDropNonemptyNamespaceError(namespace: Seq[String]): Throwable = { - new SparkException( - s"Cannot drop a non-empty namespace: ${namespace.quoted}. " + - "Use CASCADE option to drop a non-empty namespace.") - } - def noRecordsFromEmptyDataReaderError(): Throwable = { new IOException("No records should be returned from EmptyDataReader") } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DropNamespaceExec.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DropNamespaceExec.scala index dbd5cbd874945..9a9d8e1d4d57d 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DropNamespaceExec.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DropNamespaceExec.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.execution.datasources.v2 import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.catalyst.expressions.Attribute import org.apache.spark.sql.connector.catalog.CatalogPlugin -import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors} +import org.apache.spark.sql.errors.QueryCompilationErrors /** * Physical plan node for dropping a namespace. @@ -42,12 +42,12 @@ case class DropNamespaceExec( if (!cascade) { if (catalog.asTableCatalog.listTables(ns).nonEmpty || nsCatalog.listNamespaces(ns).nonEmpty) { - throw QueryExecutionErrors.cannotDropNonemptyNamespaceError(namespace) + throw QueryCompilationErrors.cannotDropNonemptyNamespaceError(namespace) } } if (!nsCatalog.dropNamespace(ns)) { - throw QueryExecutionErrors.cannotDropNonemptyNamespaceError(namespace) + throw QueryCompilationErrors.cannotDropNonemptyNamespaceError(namespace) } } else if (!ifExists) { throw QueryCompilationErrors.noSuchNamespaceError(ns) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DropNamespaceSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DropNamespaceSuiteBase.scala index 1ff4e1bac049f..376f376c32d5b 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DropNamespaceSuiteBase.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DropNamespaceSuiteBase.scala @@ -36,7 +36,7 @@ trait DropNamespaceSuiteBase extends QueryTest with DDLCommandTestUtils { protected def builtinTopNamespaces: Seq[String] = Seq.empty protected def isCasePreserving: Boolean = true - protected def assertDropFails(): Unit + protected def namespaceAlias: String = "namespace" protected def checkNamespace(expected: Seq[String]) = { val df = spark.sql(s"SHOW NAMESPACES IN $catalog") @@ -72,7 +72,10 @@ trait DropNamespaceSuiteBase extends QueryTest with DDLCommandTestUtils { checkNamespace(Seq("ns") ++ builtinTopNamespaces) // $catalog.ns.table is present, thus $catalog.ns cannot be dropped. - assertDropFails() + val e = intercept[AnalysisException] { + sql(s"DROP NAMESPACE $catalog.ns") + } + assert(e.getMessage.contains(s"Cannot drop a non-empty $namespaceAlias: ns")) sql(s"DROP TABLE $catalog.ns.table") // Now that $catalog.ns is empty, it can be dropped. diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DropNamespaceSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DropNamespaceSuite.scala index 2db4423f1be37..24e51317575d3 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DropNamespaceSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/DropNamespaceSuite.scala @@ -31,12 +31,7 @@ import org.apache.spark.sql.execution.command trait DropNamespaceSuiteBase extends command.DropNamespaceSuiteBase { override protected def builtinTopNamespaces: Seq[String] = Seq("default") - override protected def assertDropFails(): Unit = { - val e = intercept[AnalysisException] { - sql(s"DROP NAMESPACE $catalog.ns") - } - assert(e.getMessage.contains("Database ns is not empty. One or more tables exist")) - } + override protected def namespaceAlias(): String = "database" test("drop default namespace") { val message = intercept[AnalysisException] { diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/DropNamespaceSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/DropNamespaceSuite.scala index ddc913e0d80b9..6f82ee5827eda 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/DropNamespaceSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/DropNamespaceSuite.scala @@ -17,18 +17,9 @@ package org.apache.spark.sql.execution.command.v2 -import org.apache.spark.SparkException import org.apache.spark.sql.execution.command /** * The class contains tests for the `DROP NAMESPACE` command to check V2 table catalogs. */ -class DropNamespaceSuite extends command.DropNamespaceSuiteBase with CommandSuiteBase { - // TODO: Unify the error that throws from v1 and v2 test suite into `AnalysisException` - override protected def assertDropFails(): Unit = { - val e = intercept[SparkException] { - sql(s"DROP NAMESPACE $catalog.ns") - } - assert(e.getMessage.contains("Cannot drop a non-empty namespace: ns")) - } -} +class DropNamespaceSuite extends command.DropNamespaceSuiteBase with CommandSuiteBase diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala index 5fccce2678f86..525454b366d1e 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala @@ -196,7 +196,15 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat db: String, ignoreIfNotExists: Boolean, cascade: Boolean): Unit = withClient { - client.dropDatabase(db, ignoreIfNotExists, cascade) + try { + client.dropDatabase(db, ignoreIfNotExists, cascade) + } catch { + case NonFatal(exception) => + if (exception.getClass.getName.equals("org.apache.hadoop.hive.ql.metadata.HiveException") + && exception.getMessage.contains(s"Database $db is not empty.")) { + throw new AnalysisException(s"Cannot drop a non-empty database: $db") + } else throw exception + } } /** From 00e7ab54c8d52336f6a4688292e1fa8c36656cac Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Fri, 24 Dec 2021 13:58:18 +0700 Subject: [PATCH 2/7] fix hive test suite for new error message --- .../scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala | 2 +- .../org/apache/spark/sql/hive/execution/HiveDDLSuite.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala index 525454b366d1e..5d37457d6b567 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala @@ -202,7 +202,7 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat case NonFatal(exception) => if (exception.getClass.getName.equals("org.apache.hadoop.hive.ql.metadata.HiveException") && exception.getMessage.contains(s"Database $db is not empty.")) { - throw new AnalysisException(s"Cannot drop a non-empty database: $db") + throw new AnalysisException(s"Cannot drop a non-empty database: $db.") } else throw exception } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala index 3bf3ee58082eb..014feb33df5ea 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala @@ -1235,7 +1235,7 @@ class HiveDDLSuite if (tableExists && !cascade) { assertAnalysisError( sqlDropDatabase, - s"Database $dbName is not empty. One or more tables exist.") + s"Cannot drop a non-empty database: $dbName.") // the database directory was not removed assert(fs.exists(new Path(expectedDBLocation))) } else { From 2a237043f1bd49f6618848126e41f89ff0cb5191 Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Mon, 27 Dec 2021 17:39:02 +0700 Subject: [PATCH 3/7] update v1 error message --- .../apache/spark/sql/errors/QueryCompilationErrors.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala index 317b629bd1d77..0f79facafe7c2 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala @@ -541,12 +541,12 @@ object QueryCompilationErrors { } def cannotDropNonemptyDatabaseError(db: String, details: String): Throwable = { - new AnalysisException(s"Cannot drop a non-empty database: $db. One or more $details exist.") + new AnalysisException(s"Cannot drop a non-empty database: $db. " + + "Use CASCADE option to drop a non-empty database.") } def cannotDropNonemptyNamespaceError(namespace: Seq[String]): Throwable = { - new AnalysisException( - s"Cannot drop a non-empty namespace: ${namespace.quoted}. " + + new AnalysisException(s"Cannot drop a non-empty namespace: ${namespace.quoted}. " + "Use CASCADE option to drop a non-empty namespace.") } From 3ebdc9cf279617034b24f8a0e338c6d5c06f86ca Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Tue, 28 Dec 2021 08:51:42 +0700 Subject: [PATCH 4/7] update v1 error message for CASCADE --- .../org/apache/spark/sql/errors/QueryCompilationErrors.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala index 0f79facafe7c2..bcb7cba229bb7 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala @@ -542,7 +542,7 @@ object QueryCompilationErrors { def cannotDropNonemptyDatabaseError(db: String, details: String): Throwable = { new AnalysisException(s"Cannot drop a non-empty database: $db. " + - "Use CASCADE option to drop a non-empty database.") + s"One or more $details exist. Use CASCADE option to drop a non-empty database.") } def cannotDropNonemptyNamespaceError(namespace: Seq[String]): Throwable = { From d6951cda634a2e1a30bb5d9ab666d26c577f7bd0 Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Tue, 28 Dec 2021 10:56:58 +0700 Subject: [PATCH 5/7] resolve review --- .../apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala | 4 ++-- .../apache/spark/sql/errors/QueryCompilationErrors.scala | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala index 9cc727e297cef..1557cf4b289f1 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala @@ -140,10 +140,10 @@ class InMemoryCatalog( if (!cascade) { // If cascade is false, make sure the database is empty. if (catalog(db).tables.nonEmpty) { - throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db, "tables") + throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db) } if (catalog(db).functions.nonEmpty) { - throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db, "functions") + throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db) } } // Remove the database. diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala index bcb7cba229bb7..c948b70940f9b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala @@ -540,14 +540,14 @@ object QueryCompilationErrors { s"rename temporary view from '$oldName' to '$newName': destination view already exists") } - def cannotDropNonemptyDatabaseError(db: String, details: String): Throwable = { + def cannotDropNonemptyDatabaseError(db: String): Throwable = { new AnalysisException(s"Cannot drop a non-empty database: $db. " + - s"One or more $details exist. Use CASCADE option to drop a non-empty database.") + "Use CASCADE option to drop a non-empty database.") } def cannotDropNonemptyNamespaceError(namespace: Seq[String]): Throwable = { new AnalysisException(s"Cannot drop a non-empty namespace: ${namespace.quoted}. " + - "Use CASCADE option to drop a non-empty namespace.") + "Use CASCADE option to drop a non-empty namespace.") } def invalidNameForTableOrDatabaseError(name: String): Throwable = { From 941207d2bdc37f772ffa1a42050cc764bdbdd15f Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Tue, 28 Dec 2021 11:01:07 +0700 Subject: [PATCH 6/7] update --- .../apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala index 1557cf4b289f1..e3896c598eac9 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala @@ -139,10 +139,7 @@ class InMemoryCatalog( if (catalog.contains(db)) { if (!cascade) { // If cascade is false, make sure the database is empty. - if (catalog(db).tables.nonEmpty) { - throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db) - } - if (catalog(db).functions.nonEmpty) { + if (catalog(db).tables.nonEmpty || catalog(db).functions.nonEmpty) { throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db) } } From 17beaa839b9f04a8100a4e942478825757382dc1 Mon Sep 17 00:00:00 2001 From: dch nguyen Date: Wed, 29 Dec 2021 11:04:48 +0700 Subject: [PATCH 7/7] use cannotDropNonemptyDatabaseError for hive catalog test --- .../scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala index 5d37457d6b567..52b21598edc09 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala @@ -41,6 +41,7 @@ import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.catalog.ExternalCatalogUtils._ import org.apache.spark.sql.catalyst.expressions._ import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap, CharVarcharUtils} +import org.apache.spark.sql.errors.QueryCompilationErrors import org.apache.spark.sql.execution.command.DDLUtils import org.apache.spark.sql.execution.datasources.{PartitioningUtils, SourceOptions} import org.apache.spark.sql.hive.client.HiveClient @@ -202,7 +203,7 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat case NonFatal(exception) => if (exception.getClass.getName.equals("org.apache.hadoop.hive.ql.metadata.HiveException") && exception.getMessage.contains(s"Database $db is not empty.")) { - throw new AnalysisException(s"Cannot drop a non-empty database: $db.") + throw QueryCompilationErrors.cannotDropNonemptyDatabaseError(db) } else throw exception } }