cloud-fan commented on a change in pull request #25363: [SPARK-28628][SQL] 
Implement SupportsNamespaces in V2SessionCatalog
URL: https://github.com/apache/spark/pull/25363#discussion_r319812223
 
 

 ##########
 File path: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala
 ##########
 @@ -753,3 +760,293 @@ class V2SessionCatalogSuite
     assert(exc.message.contains("RENAME TABLE source and destination databases 
do not match"))
   }
 }
+
+class V2SessionCatalogNamespaceSuite extends V2SessionCatalogBaseSuite {
+
+  import org.apache.spark.sql.catalog.v2.CatalogV2Implicits._
+
+  def checkMetadata(
+      expected: scala.collection.Map[String, String],
+      actual: scala.collection.Map[String, String]): Unit = {
+    // remove location and comment that are automatically added by HMS unless 
they are expected
+    val toRemove = 
V2SessionCatalog.RESERVED_PROPERTIES.filter(expected.contains)
+    assert(expected -- toRemove === actual)
+  }
+
+  test("listNamespaces: basic behavior") {
+    val catalog = newCatalog()
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    assert(catalog.listNamespaces() === Array(testNs, defaultNs))
+    assert(catalog.listNamespaces(Array()) === Array(testNs, defaultNs))
+    assert(catalog.listNamespaces(testNs) === Array())
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("listNamespaces: fail if missing namespace") {
+    val catalog = newCatalog()
+
+    assert(catalog.namespaceExists(testNs) === false)
+
+    val exc = intercept[NoSuchNamespaceException] {
+      assert(catalog.listNamespaces(testNs) === Array())
+    }
+
+    assert(exc.getMessage.contains(testNs.quoted))
+    assert(catalog.namespaceExists(testNs) === false)
+  }
+
+  test("loadNamespaceMetadata: fail missing namespace") {
+    val catalog = newCatalog()
+
+    val exc = intercept[NoSuchNamespaceException] {
+      catalog.loadNamespaceMetadata(testNs)
+    }
+
+    assert(exc.getMessage.contains(testNs.quoted))
+  }
+
+  test("loadNamespaceMetadata: non-empty metadata") {
+    val catalog = newCatalog()
+
+    assert(catalog.namespaceExists(testNs) === false)
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    val metadata = catalog.loadNamespaceMetadata(testNs)
+
+    assert(catalog.namespaceExists(testNs) === true)
+    checkMetadata(metadata.asScala, Map("property" -> "value"))
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("loadNamespaceMetadata: empty metadata") {
+    val catalog = newCatalog()
+
+    assert(catalog.namespaceExists(testNs) === false)
+
+    catalog.createNamespace(testNs, emptyProps)
+
+    val metadata = catalog.loadNamespaceMetadata(testNs)
+
+    assert(catalog.namespaceExists(testNs) === true)
+    checkMetadata(metadata.asScala, emptyProps.asScala)
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("createNamespace: basic behavior") {
+    val catalog = newCatalog()
+    val expectedPath = 
sqlContext.sessionState.catalog.getDefaultDBPath(testNs(0)).toString
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    assert(expectedPath === 
spark.catalog.getDatabase(testNs(0)).locationUri.toString)
+
+    assert(catalog.namespaceExists(testNs) === true)
+    val metadata = catalog.loadNamespaceMetadata(testNs).asScala
+    checkMetadata(metadata, Map("property" -> "value"))
+    assert(expectedPath === metadata("location"))
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("createNamespace: initialize location") {
+    val catalog = newCatalog()
+    val expectedPath = "file:/tmp/db.db"
+
+    catalog.createNamespace(testNs, Map("location" -> expectedPath).asJava)
+
+    assert(expectedPath === 
spark.catalog.getDatabase(testNs(0)).locationUri.toString)
+
+    assert(catalog.namespaceExists(testNs) === true)
+    val metadata = catalog.loadNamespaceMetadata(testNs).asScala
+    checkMetadata(metadata, Map.empty)
+    assert(expectedPath === metadata("location"))
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("createNamespace: fail if namespace already exists") {
+    val catalog = newCatalog()
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    val exc = intercept[NamespaceAlreadyExistsException] {
+      catalog.createNamespace(testNs, Map("property" -> "value2").asJava)
+    }
+
+    assert(exc.getMessage.contains(testNs.quoted))
+    assert(catalog.namespaceExists(testNs) === true)
+    checkMetadata(catalog.loadNamespaceMetadata(testNs).asScala, 
Map("property" -> "value"))
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("createNamespace: fail nested namespace") {
+    val catalog = newCatalog()
+
+    // ensure the parent exists
+    catalog.createNamespace(Array("db"), emptyProps)
+
+    val exc = intercept[IllegalArgumentException] {
+      catalog.createNamespace(Array("db", "nested"), emptyProps)
+    }
+
+    assert(exc.getMessage.contains("Invalid namespace name: db.nested"))
+
+    catalog.dropNamespace(Array("db"))
+  }
+
+  test("createTable: fail if namespace does not exist") {
+    val catalog = newCatalog()
+
+    assert(catalog.namespaceExists(testNs) === false)
+
+    val exc = intercept[NoSuchNamespaceException] {
+      catalog.createTable(testIdent, schema, Array.empty, emptyProps)
+    }
+
+    assert(exc.getMessage.contains(testNs.quoted))
+    assert(catalog.namespaceExists(testNs) === false)
+  }
+
+  test("dropNamespace: drop missing namespace") {
+    val catalog = newCatalog()
+
+    assert(catalog.namespaceExists(testNs) === false)
+
+    val ret = catalog.dropNamespace(testNs)
+
+    assert(ret === false)
+  }
+
+  test("dropNamespace: drop empty namespace") {
+    val catalog = newCatalog()
+
+    catalog.createNamespace(testNs, emptyProps)
+
+    assert(catalog.namespaceExists(testNs) === true)
+
+    val ret = catalog.dropNamespace(testNs)
+
+    assert(ret === true)
+    assert(catalog.namespaceExists(testNs) === false)
+  }
+
+  test("dropNamespace: fail if not empty") {
+    val catalog = newCatalog()
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+    catalog.createTable(testIdent, schema, Array.empty, emptyProps)
+
+    val exc = intercept[IllegalStateException] {
+      catalog.dropNamespace(testNs)
+    }
+
+    assert(exc.getMessage.contains(testNs.quoted))
+    assert(catalog.namespaceExists(testNs) === true)
+    checkMetadata(catalog.loadNamespaceMetadata(testNs).asScala, 
Map("property" -> "value"))
+
+    catalog.dropTable(testIdent)
+    catalog.dropNamespace(testNs)
+  }
+
+  test("alterNamespace: basic behavior") {
+    val catalog = newCatalog()
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    catalog.alterNamespace(testNs, NamespaceChange.setProperty("property2", 
"value2"))
+    checkMetadata(
+      catalog.loadNamespaceMetadata(testNs).asScala,
+      Map("property" -> "value", "property2" -> "value2"))
+
+    catalog.alterNamespace(testNs,
+      NamespaceChange.removeProperty("property2"),
+      NamespaceChange.setProperty("property3", "value3"))
+    checkMetadata(
+      catalog.loadNamespaceMetadata(testNs).asScala,
+      Map("property" -> "value", "property3" -> "value3"))
+
+    catalog.alterNamespace(testNs, NamespaceChange.removeProperty("property3"))
+    checkMetadata(
+      catalog.loadNamespaceMetadata(testNs).asScala,
+      Map("property" -> "value"))
+
+    catalog.dropNamespace(testNs)
+  }
+
+  test("alterNamespace: update namespace location") {
+    val catalog = newCatalog()
+    val initialPath = 
sqlContext.sessionState.catalog.getDefaultDBPath(testNs(0)).toString
+    val newPath = "file:/tmp/db.db"
+
+    catalog.createNamespace(testNs, emptyProps)
+
+    assert(initialPath === 
spark.catalog.getDatabase(testNs(0)).locationUri.toString)
+
+    catalog.alterNamespace(testNs, NamespaceChange.setProperty("location", 
newPath))
 
 Review comment:
   Hive supports it: https://issues.apache.org/jira/browse/HIVE-8472
   
   However, AFAIK Spark has a problem to support it: 
https://github.com/apache/spark/pull/25294
   
   @rdblue can we check if the newly created tables under this namespace can 
reflect the location change?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to