Repository: spark Updated Branches: refs/heads/branch-2.1 fc1b25660 -> c1a26b458
[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 <wenc...@databricks.com> Closes #16332 from cloud-fan/minor. (cherry picked from commit 7a75ee1c9224aa5c2e954fe2a71f9ad506f6782b) Signed-off-by: Yin Huai <yh...@databricks.com> Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/c1a26b45 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/c1a26b45 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/c1a26b45 Branch: refs/heads/branch-2.1 Commit: c1a26b458dd353be3ab1a2b3f9bb80809cf63479 Parents: fc1b256 Author: Wenchen Fan <wenc...@databricks.com> Authored: Mon Dec 19 11:42:59 2016 -0800 Committer: Yin Huai <yh...@databricks.com> Committed: Mon Dec 19 11:43:55 2016 -0800 ---------------------------------------------------------------------- .../apache/spark/sql/hive/HiveExternalCatalog.scala | 2 +- .../org/apache/spark/sql/hive/client/HiveClient.scala | 8 +++----- .../apache/spark/sql/hive/client/HiveClientImpl.scala | 12 ++++++++---- .../apache/spark/sql/hive/client/VersionsSuite.scala | 13 +++++++------ 4 files changed, 19 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/c1a26b45/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---------------------------------------------------------------------- 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 f67ddc9..f321c45 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 { http://git-wip-us.apache.org/repos/asf/spark/blob/c1a26b45/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala ---------------------------------------------------------------------- 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 8e7c871..0be5b0b 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] http://git-wip-us.apache.org/repos/asf/spark/blob/c1a26b45/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---------------------------------------------------------------------- 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 db73596..e0f7156 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 { http://git-wip-us.apache.org/repos/asf/spark/blob/c1a26b45/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala ---------------------------------------------------------------------- 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 bfec430..e706e2e 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) } /////////////////////////////////////////////////////////////////////////// --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org
