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

 ##########
 File path: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala
 ##########
 @@ -753,3 +759,198 @@ class V2SessionCatalogSuite
     assert(exc.message.contains("RENAME TABLE source and destination databases 
do not match"))
   }
 }
+
+class V2SessionCatalogNamespaceSuite extends V2SessionCatalogSuite {
+
+  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 = Seq("location", "comment").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()
+
+    catalog.createNamespace(testNs, Map("property" -> "value").asJava)
+
+    assert(catalog.namespaceExists(testNs) === true)
+    checkMetadata(catalog.loadNamespaceMetadata(testNs).asScala, 
Map("property" -> "value"))
+
+    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("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") {
 
 Review comment:
   I added tests that fail to remove reserved properties, location and comment, 
because the catalog requires values. I also added tests that validate they can 
be updated by altering the namespace to set those properties.
   
   This isn't actually exposed yet because there is no SQL or public API for 
altering a namespace. But this is how table properties work, so I think it 
makes sense to match the behavior.

----------------------------------------------------------------
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