You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/02 23:07:50 UTC

[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #36750: [SPARK-29260][SQL] Support alter database location for Hive client versions other than 3.0/3.1

dongjoon-hyun commented on code in PR #36750:
URL: https://github.com/apache/spark/pull/36750#discussion_r888476932


##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala:
##########
@@ -165,19 +165,19 @@ class HiveClientSuite(version: String, allVersions: Seq[String])
     assert(client.getDatabase("temporary").properties.contains("flag"))
 
     // test alter database location
+    val oldDatabasePath = database.locationUri.getPath
     val tempDatabasePath2 = Utils.createTempDir().toURI
-    // Hive support altering database location since HIVE-8472.
+    client.alterDatabase(database.copy(locationUri = tempDatabasePath2))
+    val uriInCatalog = client.getDatabase("temporary").locationUri
+    assert("file" === uriInCatalog.getScheme)
+
     if (version == "3.0" || version == "3.1") {
-      client.alterDatabase(database.copy(locationUri = tempDatabasePath2))
-      val uriInCatalog = client.getDatabase("temporary").locationUri
-      assert("file" === uriInCatalog.getScheme)
+      // Hive support altering database location since HIVE-8472
       assert(new Path(tempDatabasePath2.getPath).toUri.getPath === uriInCatalog.getPath,
         "Failed to alter database location")
     } else {
-      val e = intercept[AnalysisException] {
-        client.alterDatabase(database.copy(locationUri = tempDatabasePath2))
-      }
-      assert(e.getMessage.contains("does not support altering database location"))
+      // .. otherwise, the command should be non-effective against older versions of Hive
+      assert(oldDatabasePath === uriInCatalog.getPath, "Expected database location to be unchanged")

Review Comment:
   If there is no exception from HMS side in this case, we had better give a warning from Spark side here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org