From c1a26b458dd353be3ab1a2b3f9bb80809cf63479 Mon Sep 17 00:00:00 2001
From: Wenchen Fan <wenchen@databricks.com>
Date: Mon, 19 Dec 2016 11:42:59 -0800
Subject: [PATCH] [SPARK-18921][SQL] check database existence with
Hive.databaseExists instead of getDatabase
## What changes were proposed in this pull request?
It's weird that we use `Hive.getDatabase` to check the existence of a database, while Hive has a `databaseExists` interface.
What's worse, `Hive.getDatabase` will produce an error message if the database doesn't exist, which is annoying when we only want to check the database existence.
This PR fixes this and use `Hive.databaseExists` to check database existence.
## How was this patch tested?
N/A
Author: Wenchen Fan <wenchen@databricks.com>
Closes #16332 from cloud-fan/minor.
(cherry picked from commit 7a75ee1c9224aa5c2e954fe2a71f9ad506f6782b)
Signed-off-by: Yin Huai <yhuai@databricks.com>
---
.../apache/spark/sql/hive/HiveExternalCatalog.scala | 2 +-
.../apache/spark/sql/hive/client/HiveClient.scala | 8 +++-----
.../spark/sql/hive/client/HiveClientImpl.scala | 12 ++++++++----
.../spark/sql/hive/client/VersionsSuite.scala | 13 +++++++------
4 files changed, 19 insertions(+), 16 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 f67ddc9be1..f321c45e5c 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
@@ -167,7 +167,7 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
}
override def databaseExists(db: String): Boolean = withClient {
- client.getDatabaseOption(db).isDefined
+ client.databaseExists(db)
}
override def listDatabases(): Seq[String] = withClient {
diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala
index 8e7c871183..0be5b0bedf 100644
--- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala
+++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala
@@ -58,12 +58,10 @@ private[hive] trait HiveClient {
def setCurrentDatabase(databaseName: String): Unit
/** Returns the metadata for specified database, throwing an exception if it doesn't exist */
- final def getDatabase(name: String): CatalogDatabase = {
- getDatabaseOption(name).getOrElse(throw new NoSuchDatabaseException(name))
- }
+ def getDatabase(name: String): CatalogDatabase
- /** Returns the metadata for a given database, or None if it doesn't exist. */
- def getDatabaseOption(name: String): Option[CatalogDatabase]
+ /** Return whether a table/view with the specified name exists. */
+ def databaseExists(dbName: String): Boolean
/** List the names of all the databases that match the specified pattern. */
def listDatabases(pattern: String): Seq[String]
diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
index db73596e5f..e0f71560f3 100644
--- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
+++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
@@ -300,7 +300,7 @@ private[hive] class HiveClientImpl(
}
override def setCurrentDatabase(databaseName: String): Unit = withHiveState {
- if (getDatabaseOption(databaseName).isDefined) {
+ if (databaseExists(databaseName)) {
state.setCurrentDatabase(databaseName)
} else {
throw new NoSuchDatabaseException(databaseName)
@@ -336,14 +336,18 @@ private[hive] class HiveClientImpl(
Option(database.properties).map(_.asJava).orNull))
}
- override def getDatabaseOption(name: String): Option[CatalogDatabase] = withHiveState {
- Option(client.getDatabase(name)).map { d =>
+ override def getDatabase(dbName: String): CatalogDatabase = withHiveState {
+ Option(client.getDatabase(dbName)).map { d =>
CatalogDatabase(
name = d.getName,
description = d.getDescription,
locationUri = d.getLocationUri,
properties = Option(d.getParameters).map(_.asScala.toMap).orNull)
- }
+ }.getOrElse(throw new NoSuchDatabaseException(dbName))
+ }
+
+ override def databaseExists(dbName: String): Boolean = withHiveState {
+ client.databaseExists(dbName)
}
override def listDatabases(pattern: String): Seq[String] = withHiveState {
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala
index bfec43070a..e706e2eb1f 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala
@@ -28,7 +28,7 @@ import org.apache.spark.SparkFunSuite
import org.apache.spark.internal.Logging
import org.apache.spark.sql.{AnalysisException, Row}
import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
-import org.apache.spark.sql.catalyst.analysis.NoSuchPermanentFunctionException
+import org.apache.spark.sql.catalyst.analysis.{NoSuchDatabaseException, NoSuchPermanentFunctionException}
import org.apache.spark.sql.catalyst.catalog._
import org.apache.spark.sql.catalyst.expressions.{AttributeReference, EqualTo, Literal}
import org.apache.spark.sql.catalyst.util.quietly
@@ -137,11 +137,12 @@ class VersionsSuite extends SparkFunSuite with SQLTestUtils with TestHiveSinglet
test(s"$version: getDatabase") {
// No exception should be thrown
client.getDatabase("default")
+ intercept[NoSuchDatabaseException](client.getDatabase("nonexist"))
}
- test(s"$version: getDatabaseOption") {
- assert(client.getDatabaseOption("default").isDefined)
- assert(client.getDatabaseOption("nonexist") == None)
+ test(s"$version: databaseExists") {
+ assert(client.databaseExists("default") == true)
+ assert(client.databaseExists("nonexist") == false)
}
test(s"$version: listDatabases") {
@@ -155,9 +156,9 @@ class VersionsSuite extends SparkFunSuite with SQLTestUtils with TestHiveSinglet
}
test(s"$version: dropDatabase") {
- assert(client.getDatabaseOption("temporary").isDefined)
+ assert(client.databaseExists("temporary") == true)
client.dropDatabase("temporary", ignoreIfNotExists = false, cascade = true)
- assert(client.getDatabaseOption("temporary").isEmpty)
+ assert(client.databaseExists("temporary") == false)
}
///////////////////////////////////////////////////////////////////////////
--
GitLab