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 2020/11/23 18:29:39 UTC
[GitHub] [spark] huaxingao opened a new pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
huaxingao opened a new pull request #30473:
URL: https://github.com/apache/spark/pull/30473
### What changes were proposed in this pull request?
Add namespaces support in JDBC v2 Table Catalog by making ```JDBCTableCatalog``` extends```SupportsNamespaces```
### Why are the changes needed?
make v2 JDBC implementation complete
### Does this PR introduce _any_ user-facing change?
Yes. Add the following to ```JDBCTableCatalog```
- listNamespaces
- listNamespaces(String[] namespace)
- namespaceExists(String[] namespace)
- loadNamespaceMetadata(String[] namespace)
- createNamespace
- alterNamespace
- dropNamespace
### How was this patch tested?
Add new docker tests
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737573729
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732724340
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r535187274
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCNamespaceTest.scala
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.jdbc.v2
+
+import scala.collection.JavaConverters._
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.connector.catalog.NamespaceChange
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+private[v2] trait V2JDBCNamespaceTest extends SharedSparkSession {
+ val catalog = new JDBCTableCatalog()
+
+ def testListNamespaces: Unit
Review comment:
how about `def builtinNamespaces: Seq[Seq[String]]`
then in test
```
assert(catalog.listNamespaces() === Array(Array("foo")) ++ builtinNamespaces)
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735605839
**[Test build #131966 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131966/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r534522414
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
Review comment:
DB2 jdbc driver implements this to return schemas from the current catalog (the current database jdbc driver connects to). Not sure how other jdbc drivers implement this. I tested with postgres jdbc driver, it also returns schemas from the current catalog. In listTables (https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala#L62), we also assume ```getTables``` with `null` as catalog value returns tables from the current catalog.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738161301
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36746/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533375795
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
Review comment:
A very easy way is to use `conn.getMetaData.getSchemas(catalog, db)` to avoid listing all databases.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732347117
**[Test build #131567 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131567/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737533821
**[Test build #132073 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132073/testReport)** for PR 30473 at commit [`8317cd8`](https://github.com/apache/spark/commit/8317cd8cca16ac54ccd517ba5a5a1446efc91e79).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732934562
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732536777
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738161328
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r530068393
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
Review comment:
does JDBC v2 connect to a certain catalog?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732397837
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36168/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738237581
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532430050
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ catalogs.next()
+ val catalog = catalogs.getString(1)
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val result = conn.getMetaData.getSchemas(catalog, null)
+ while(result.next()) {
+ schemaBuilder += Array(result.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if listNamespaces.exists (_(0) == db) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(_) =>
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !listNamespaces.exists (_(0) == db) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case "comment" => comment = v
+ case "owner" => // ignore
+ case "location" =>
+ throw new AnalysisException("CREATE CATALOG ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
+ if (set.property() == "comment") {
+ withConnection { conn =>
+ JdbcUtils.createNamespaceComment(conn, options, db, set.value)
+ }
+ }
+
+ case unset: NamespaceChange.RemoveProperty =>
+ // ignore changes other than comments
+ if (unset.property() == "comment") {
+ withConnection { conn =>
+ JdbcUtils.removeNamespaceComment(conn, options, db)
+ }
+ }
+
+ case _ =>
Review comment:
shall we fail and say "action xxx not supported"?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737573729
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r534522656
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
+ if (set.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.createNamespaceComment(conn, options, db, set.value)
+ }
+ }
+
+ case unset: NamespaceChange.RemoveProperty =>
+ // ignore changes other than comments
+ if (unset.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.removeNamespaceComment(conn, options, db)
+ }
+ }
+
+ case _ =>
+ throw new SQLFeatureNotSupportedException(s"Unsupported NamespaceChange $changes")
+ }
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) if namespaceExists(namespace) =>
+ if (listTables(Array(db)).nonEmpty) {
+ throw new IllegalStateException(s"Namespace ${namespace.quoted} is not empty")
+ }
+ withConnection { conn =>
+ classifyException(s"Failed drop name space: $db") {
+ JdbcUtils.dropNamespace(conn, options, db)
+ true
+ }
+ }
+
+ case Array(_) =>
+ // exists returned false
Review comment:
I think we should fail. Fixed.
I was following the implementation in `V2SessionCatalog` (https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala#L273). Do I need to change this too?
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCNamespaceTest.scala
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.jdbc.v2
+
+import scala.collection.JavaConverters._
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.connector.catalog.NamespaceChange
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+private[v2] trait V2JDBCNamespaceTest extends SharedSparkSession {
+ val catalog = new JDBCTableCatalog()
+
+ test("listNamespaces: basic behavior") {
+ catalog.createNamespace(Array("foo"), Map("comment" -> "test comment").asJava)
+ assert(catalog.listNamespaces() ===
+ Array(Array("foo"), Array("information_schema"), Array("pg_catalog"), Array("public")))
Review comment:
Fixed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738621240
Thank you!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737552315
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36672/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735696537
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r534522476
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
Review comment:
I am following the implementation in `V2SessionCatalog` (https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala#L211). According to the method definition `List namespaces in a namespace`, the method returns namespaces inside the input `namespace`, which is empty.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
Review comment:
Sure. Fixed this along with the others
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533909747
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
Review comment:
shall we return the input `namespace` 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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732397862
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533910169
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
Review comment:
shall we fail to match the behavior of unknown table properties?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533910906
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
+ if (set.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.createNamespaceComment(conn, options, db, set.value)
+ }
+ }
+
+ case unset: NamespaceChange.RemoveProperty =>
+ // ignore changes other than comments
+ if (unset.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.removeNamespaceComment(conn, options, db)
+ }
+ }
+
+ case _ =>
+ throw new SQLFeatureNotSupportedException(s"Unsupported NamespaceChange $changes")
+ }
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) if namespaceExists(namespace) =>
+ if (listTables(Array(db)).nonEmpty) {
+ throw new IllegalStateException(s"Namespace ${namespace.quoted} is not empty")
+ }
+ withConnection { conn =>
+ classifyException(s"Failed drop name space: $db") {
+ JdbcUtils.dropNamespace(conn, options, db)
+ true
+ }
+ }
+
+ case Array(_) =>
+ // exists returned false
Review comment:
and shall we fail?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732431019
**[Test build #131567 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131567/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging `
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735696537
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732431935
Merged build finished. Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735677595
**[Test build #131966 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131966/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging `
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737587350
**[Test build #132073 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132073/testReport)** for PR 30473 at commit [`8317cd8`](https://github.com/apache/spark/commit/8317cd8cca16ac54ccd517ba5a5a1446efc91e79).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732385178
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36168/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732638289
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532425942
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ catalogs.next()
Review comment:
shall we check that `catalogs` only have one element? otherwise it's weird to see we pick the first catalog only.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732633840
**[Test build #131580 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131580/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging `
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732638289
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732536777
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732653692
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532375082
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
Review comment:
Sorry for the late reply. I was OoO last week.
Yes, it connects to the current database, which is specified in the connection url.
For example:
```postgres``` in ```jdbc:postgresql://127.0.0.1:5432/postgres;user=testUser;password=testPass```
or ```testdb0``` in ```jdbc:h2:mem:testdb0;user=testUser;password=testPass```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738113656
**[Test build #132145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132145/testReport)** for PR 30473 at commit [`68838c9`](https://github.com/apache/spark/commit/68838c979de47d35d1df3324b3a92ab80615d053).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532429029
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ catalogs.next()
+ val catalog = catalogs.getString(1)
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val result = conn.getMetaData.getSchemas(catalog, null)
+ while(result.next()) {
+ schemaBuilder += Array(result.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if listNamespaces.exists (_(0) == db) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(_) =>
Review comment:
shall we make sure it exists?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732431945
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131567/
Test FAILed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732654745
**[Test build #131620 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131620/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532429577
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ catalogs.next()
+ val catalog = catalogs.getString(1)
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val result = conn.getMetaData.getSchemas(catalog, null)
+ while(result.next()) {
+ schemaBuilder += Array(result.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if listNamespaces.exists (_(0) == db) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(_) =>
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !listNamespaces.exists (_(0) == db) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case "comment" => comment = v
+ case "owner" => // ignore
+ case "location" =>
+ throw new AnalysisException("CREATE CATALOG ... LOCATION ... is not supported in" +
Review comment:
`CREATE NAMESPACE`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533910609
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
+ if (set.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.createNamespaceComment(conn, options, db, set.value)
+ }
+ }
+
+ case unset: NamespaceChange.RemoveProperty =>
+ // ignore changes other than comments
+ if (unset.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.removeNamespaceComment(conn, options, db)
+ }
+ }
+
+ case _ =>
+ throw new SQLFeatureNotSupportedException(s"Unsupported NamespaceChange $changes")
+ }
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def dropNamespace(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) if namespaceExists(namespace) =>
+ if (listTables(Array(db)).nonEmpty) {
+ throw new IllegalStateException(s"Namespace ${namespace.quoted} is not empty")
+ }
+ withConnection { conn =>
+ classifyException(s"Failed drop name space: $db") {
+ JdbcUtils.dropNamespace(conn, options, db)
+ true
+ }
+ }
+
+ case Array(_) =>
+ // exists returned false
Review comment:
not exists
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738113656
**[Test build #132145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132145/testReport)** for PR 30473 at commit [`68838c9`](https://github.com/apache/spark/commit/68838c979de47d35d1df3324b3a92ab80615d053).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738237581
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732501360
**[Test build #131580 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131580/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30473:
URL: https://github.com/apache/spark/pull/30473
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533909303
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
Review comment:
does it return schemas only from the current catalog?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r528916478
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ catalogs.next()
+ val catalog = catalogs.getString(1)
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val result = conn.getMetaData.getSchemas(catalog, null)
+ while(result.next()) {
+ schemaBuilder += Array(result.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if listNamespaces.exists (_(0) == db) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(_) =>
+ mutable.HashMap[String, String]().asJava
Review comment:
Seems to me the only property we can support for name space is schema comment. I don't have a good way to retrieve schema comment, so I will return an empty map for now.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738615919
thanks, merging to master!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737533821
**[Test build #132073 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132073/testReport)** for PR 30473 at commit [`8317cd8`](https://github.com/apache/spark/commit/8317cd8cca16ac54ccd517ba5a5a1446efc91e79).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732348636
I have problem testing the new namespace APIs using H2: after creating a new namespace, somehow I can't see it in listNamespaces. Testing against Postgres works OK. That's why I didn't add a regular JDBC test. I added a Postgres docker test instead.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732501360
**[Test build #131580 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131580/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533911399
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCNamespaceTest.scala
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.jdbc.v2
+
+import scala.collection.JavaConverters._
+
+import org.apache.log4j.Level
+
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.connector.catalog.NamespaceChange
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+private[v2] trait V2JDBCNamespaceTest extends SharedSparkSession {
+ val catalog = new JDBCTableCatalog()
+
+ test("listNamespaces: basic behavior") {
+ catalog.createNamespace(Array("foo"), Map("comment" -> "test comment").asJava)
+ assert(catalog.listNamespaces() ===
+ Array(Array("foo"), Array("information_schema"), Array("pg_catalog"), Array("public")))
Review comment:
this seems pgsql specific.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533375795
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
Review comment:
One way is to use `conn.getMetaData.getSchemas(catalog, db)` to avoid listing all databases.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532990800
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
Review comment:
You mean it's expensive to fetch namespaces info from the underlying databases, right? We can't save the previously fetched info and reuse it, because somebody else might have created or dropped namespaces after the last fetch. I guess we have to fetch again every time we need the info?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737594925
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735592504
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735605839
**[Test build #131966 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131966/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732464524
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735664310
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738161328
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737563247
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36672/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732724340
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738210526
**[Test build #132145 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132145/testReport)** for PR 30473 at commit [`68838c9`](https://github.com/apache/spark/commit/68838c979de47d35d1df3324b3a92ab80615d053).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-735664326
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-738139824
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36746/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732654745
**[Test build #131620 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131620/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533910326
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
Review comment:
ditto, shall we fail?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -171,6 +175,130 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ withConnection { conn =>
+ val rs = conn.getMetaData.getSchemas(null, db)
+ while (rs.next()) {
+ if (rs.getString(1) == db) return true;
+ }
+ false
+ }
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ val rs = conn.getMetaData.getSchemas()
+ while (rs.next()) {
+ schemaBuilder += Array(rs.getString(1))
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if namespaceExists(namespace) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!namespaceExists(namespace)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !namespaceExists(namespace) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case SupportsNamespaces.PROP_COMMENT => comment = v
+ case SupportsNamespaces.PROP_OWNER => // ignore
+ case SupportsNamespaces.PROP_LOCATION =>
+ throw new AnalysisException("CREATE NAMESPACE ... LOCATION ... is not supported in" +
+ " JDBC catalog.")
+ case _ => // ignore all the other properties for now
+ }
+ }
+ }
+ withConnection { conn =>
+ classifyException(s"Failed create name space: $db") {
+ JdbcUtils.createNamespace(conn, options, db, comment)
+ }
+ }
+
+ case Array(_) =>
+ throw new NamespaceAlreadyExistsException(namespace)
+
+ case _ =>
+ throw new IllegalArgumentException(s"Invalid namespace name: ${namespace.quoted}")
+ }
+
+ override def alterNamespace(namespace: Array[String], changes: NamespaceChange*): Unit = {
+ namespace match {
+ case Array(db) =>
+ changes.foreach {
+ case set: NamespaceChange.SetProperty =>
+ // ignore changes other than comments
+ if (set.property() == SupportsNamespaces.PROP_COMMENT) {
+ withConnection { conn =>
+ JdbcUtils.createNamespaceComment(conn, options, db, set.value)
+ }
+ }
+
+ case unset: NamespaceChange.RemoveProperty =>
+ // ignore changes other than comments
Review comment:
ditto
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r532426531
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,125 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
Review comment:
this seems expensive. is there a better way?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732431935
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732397862
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732711294
**[Test build #131620 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131620/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class JDBCTableCatalog extends TableCatalog with SupportsNamespaces with Logging `
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732934562
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-737594925
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30473:
URL: https://github.com/apache/spark/pull/30473#discussion_r533376591
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalog.scala
##########
@@ -170,6 +174,133 @@ class JDBCTableCatalog extends TableCatalog with Logging {
}
}
+ override def namespaceExists(namespace: Array[String]): Boolean = namespace match {
+ case Array(db) =>
+ listNamespaces.exists (_(0) == db)
+ case _ => false
+ }
+
+ override def listNamespaces(): Array[Array[String]] = {
+ withConnection { conn =>
+ val catalogs = conn.getMetaData.getCatalogs()
+ val catalogBuilder = ArrayBuilder.make[String]
+ while (catalogs.next()) {
+ val catalog = catalogs.getString(1)
+ catalogBuilder += catalog
+ }
+ val catalogArray = catalogBuilder.result;
+ val schemaBuilder = ArrayBuilder.make[Array[String]]
+ for (catalog <- catalogArray) {
+ val result = conn.getMetaData.getSchemas(catalog, null)
+ while (result.next()) {
+ schemaBuilder += Array(result.getString(1))
+ }
+ }
+ schemaBuilder.result
+ }
+ }
+
+ override def listNamespaces(namespace: Array[String]): Array[Array[String]] = {
+ namespace match {
+ case Array() =>
+ listNamespaces()
+ case Array(db) if listNamespaces.exists (_(0) == db) =>
+ Array()
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def loadNamespaceMetadata(namespace: Array[String]): util.Map[String, String] = {
+ namespace match {
+ case Array(db) =>
+ if (!listNamespaces.exists (_(0) == db)) throw new NoSuchNamespaceException(db)
+ mutable.HashMap[String, String]().asJava
+
+ case _ =>
+ throw new NoSuchNamespaceException(namespace)
+ }
+ }
+
+ override def createNamespace(
+ namespace: Array[String],
+ metadata: util.Map[String, String]): Unit = namespace match {
+ case Array(db) if !listNamespaces.exists (_(0) == db) =>
+ var comment = ""
+ if (!metadata.isEmpty) {
+ metadata.asScala.map {
+ case (k, v) => k match {
+ case "comment" => comment = v
Review comment:
can we avoid hardcoding it? `SupportsNamespaces.PROP_COMMENT`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30473: [SPARK-33430][SQL] Support namespaces in JDBC v2 Table Catalog
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30473:
URL: https://github.com/apache/spark/pull/30473#issuecomment-732347117
**[Test build #131567 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131567/testReport)** for PR 30473 at commit [`ec5f279`](https://github.com/apache/spark/commit/ec5f2799e61f4b9e50420317873fba032cf3821b).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org