-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29511][SQL] DataSourceV2: Support CREATE NAMESPACE #26166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
daa17c1
27066a5
1298b0f
08728d3
c1e6687
0fce441
6f80ff4
970338c
db5b1c9
132cd7f
3fd447d
f7fa8ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -282,6 +282,16 @@ case class InsertIntoStatement( | |
| case class ShowTablesStatement(namespace: Option[Seq[String]], pattern: Option[String]) | ||
| extends ParsedStatement | ||
|
|
||
| /** | ||
| * A CREATE NAMESPACE statement, as parsed from SQL. | ||
| */ | ||
| case class CreateNamespaceStatement( | ||
| namespace: Seq[String], | ||
|
imback82 marked this conversation as resolved.
Outdated
|
||
| ifNotExists: Boolean, | ||
| comment: Option[String], | ||
| locationSpec: Option[String], | ||
| properties: Map[String, String]) extends ParsedStatement | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cloud-fan, what is the behavior of IF NOT EXISTS with DBPROPERTIES? If the namespace exists, are properties updated?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as table/database, if table/database doesn't exist, the command is a noop. |
||
|
|
||
| /** | ||
| * A SHOW NAMESPACES statement, as parsed from SQL. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -237,6 +237,16 @@ case class ReplaceTableAsSelect( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the CREATE NAMESPACE command that works for v2 catalogs. | ||
| */ | ||
| case class CreateNamespace( | ||
| catalog: SupportsNamespaces, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: 4 space identation
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
| namespace: Seq[String], | ||
| ifNotExists: Boolean, | ||
| comment: Option[String], | ||
| locationSpec: Option[String], | ||
|
imback82 marked this conversation as resolved.
Outdated
|
||
| properties: Map[String, String]) extends Command | ||
|
|
||
| /** | ||
| * The logical plan of the SHOW NAMESPACES command that works for v2 catalogs. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -845,6 +845,63 @@ class DDLParserSuite extends AnalysisTest { | |
| ShowTablesStatement(Some(Seq("tbl")), Some("*dog*"))) | ||
| } | ||
|
|
||
| test("create namespace") { | ||
| val sql = | ||
| """ | ||
| |CREATE DATABASE IF NOT EXISTS database_name | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated. |
||
| |WITH DBPROPERTIES ('a'='a', 'b'='b', 'c'='c') | ||
| |COMMENT 'database_comment' LOCATION '/home/user/db' | ||
| """.stripMargin | ||
| comparePlans( | ||
| parsePlan(sql), | ||
| CreateNamespaceStatement( | ||
| Seq("database_name"), | ||
| ifNotExists = true, | ||
| Some("database_comment"), | ||
| Some("/home/user/db"), | ||
| Map("a" -> "a", "b" -> "b", "c" -> "c"))) | ||
| } | ||
|
|
||
| test("create namespace -- check duplicates") { | ||
| def createDatabase(duplicateClause: String): String = { | ||
| s""" | ||
| |CREATE DATABASE IF NOT EXISTS database_name | ||
| |$duplicateClause | ||
| |$duplicateClause | ||
| """.stripMargin | ||
| } | ||
| val sql1 = createDatabase("COMMENT 'database_comment'") | ||
| val sql2 = createDatabase("LOCATION '/home/user/db'") | ||
| val sql3 = createDatabase("WITH DBPROPERTIES ('a'='a', 'b'='b', 'c'='c')") | ||
|
|
||
| intercept(sql1, "Found duplicate clauses: COMMENT") | ||
| intercept(sql2, "Found duplicate clauses: LOCATION") | ||
| intercept(sql3, "Found duplicate clauses: WITH DBPROPERTIES") | ||
| } | ||
|
|
||
| test("create namespace - property values must be set") { | ||
| assertUnsupported( | ||
| sql = "CREATE DATABASE my_db WITH DBPROPERTIES('key_without_value', 'key_with_value'='x')", | ||
| containsThesePhrases = Seq("key_without_value")) | ||
| } | ||
|
|
||
| test("create namespace - support for other types in DBPROPERTIES") { | ||
| val sql = | ||
| """ | ||
| |CREATE DATABASE database_name | ||
| |LOCATION '/home/user/db' | ||
| |WITH DBPROPERTIES ('a'=1, 'b'=0.1, 'c'=TRUE) | ||
| """.stripMargin | ||
| comparePlans( | ||
| parsePlan(sql), | ||
| CreateNamespaceStatement( | ||
| Seq("database_name"), | ||
| ifNotExists = false, | ||
| None, | ||
| Some("/home/user/db"), | ||
| Map("a" -> "1", "b" -> "0.1", "c" -> "true"))) | ||
| } | ||
|
|
||
| test("show databases: basic") { | ||
| comparePlans( | ||
| parsePlan("SHOW DATABASES"), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* | ||
| * 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.execution.datasources.v2 | ||
|
|
||
| import scala.collection.JavaConverters.mapAsJavaMapConverter | ||
|
|
||
| import org.apache.spark.sql.catalyst.InternalRow | ||
| import org.apache.spark.sql.catalyst.analysis.NamespaceAlreadyExistsException | ||
| import org.apache.spark.sql.catalyst.catalog.CatalogUtils | ||
| import org.apache.spark.sql.catalyst.expressions.Attribute | ||
| import org.apache.spark.sql.connector.catalog.SupportsNamespaces | ||
|
|
||
| /** | ||
| * Physical plan node for creating a namespace. | ||
| */ | ||
| case class CreateNamespaceExec( | ||
| catalog: SupportsNamespaces, | ||
| namespace: Seq[String], | ||
| ifNotExists: Boolean, | ||
| comment: Option[String], | ||
| locationSpec: Option[String], | ||
| private var properties: Map[String, String]) | ||
| extends V2CommandExec { | ||
| override protected def run(): Seq[InternalRow] = { | ||
| if (ifNotExists && catalog.namespaceExists(namespace.toArray)) { | ||
| return Seq.empty | ||
| } | ||
|
|
||
| // Add any additional properties. | ||
|
imback82 marked this conversation as resolved.
Outdated
|
||
| if (comment.nonEmpty) { | ||
| properties += ("comment" -> comment.get) | ||
| } | ||
| if (locationSpec.nonEmpty) { | ||
| properties += ("locationSpec" -> CatalogUtils.stringToURI(locationSpec.get).toString) | ||
| } | ||
|
|
||
| catalog.createNamespace(namespace.toArray, properties.asJava) | ||
|
|
||
| Seq.empty | ||
| } | ||
|
|
||
| override def output: Seq[Attribute] = Seq.empty | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ package org.apache.spark.sql.connector | |
| import scala.collection.JavaConverters._ | ||
|
|
||
| import org.apache.spark.sql._ | ||
| import org.apache.spark.sql.catalyst.analysis.{CannotReplaceMissingTableException, NoSuchDatabaseException, NoSuchTableException, TableAlreadyExistsException} | ||
| import org.apache.spark.sql.catalyst.analysis.{CannotReplaceMissingTableException, NamespaceAlreadyExistsException, NoSuchDatabaseException, NoSuchTableException, TableAlreadyExistsException} | ||
| import org.apache.spark.sql.connector.catalog._ | ||
| import org.apache.spark.sql.connector.catalog.CatalogManager.SESSION_CATALOG_NAME | ||
| import org.apache.spark.sql.internal.SQLConf | ||
|
|
@@ -764,6 +764,32 @@ class DataSourceV2SQLSuite | |
| assert(expected === df.collect()) | ||
| } | ||
|
|
||
| test("CreateNameSpace: basic tests") { | ||
| // Session catalog is used. | ||
| sql("CREATE NAMESPACE ns") | ||
| testShowNamespaces("SHOW NAMESPACES", Seq("default", "ns")) | ||
|
|
||
| // V2 non-session catalog is used. | ||
| sql("CREATE NAMESPACE testcat.ns1.ns2") | ||
| testShowNamespaces("SHOW NAMESPACES IN testcat", Seq("ns1")) | ||
| testShowNamespaces("SHOW NAMESPACES IN testcat.ns1", Seq("ns1.ns2")) | ||
|
|
||
| // TODO: Add tests for validating namespace metadata when DESCRIBE NAMESPACE is available. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will create a JIRA for this and follow up. |
||
| } | ||
|
|
||
| test("CreateNameSpace: test handling of 'IF NOT EXIST'") { | ||
| sql("CREATE NAMESPACE IF NOT EXISTS testcat.ns1") | ||
|
|
||
| // The 'ns1' namespace already exists, so this should fail. | ||
| val exception = intercept[NamespaceAlreadyExistsException] { | ||
| sql("CREATE NAMESPACE testcat.ns1") | ||
| } | ||
| assert(exception.getMessage.contains("Namespace 'ns1' already exists")) | ||
|
|
||
| // The following will be no-op since the namespace already exists. | ||
| sql("CREATE NAMESPACE IF NOT EXISTS testcat.ns1") | ||
| } | ||
|
|
||
| test("ShowNamespaces: show root namespaces with default v2 catalog") { | ||
| spark.conf.set("spark.sql.default.catalog", "testcat") | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I missed this. Shall we use
PROPERTIESinstead ofDBPROPERTIES? We can writeDBPROPERTIES | PROPERTIESfor backward compatibilityThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for catching this. updated.