You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/03/28 21:42:10 UTC
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/12009
[SPARK-14124] [SQL] Implement Database-related DDL Commands
#### What changes were proposed in this pull request?
This PR is to implement the following four Database-related DDL commands:
- `CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name`
- `DROP DATABASE [IF EXISTS] database_name [RESTRICT|CASCADE]`
- `DESCRIBE DATABASE [EXTENDED] db_name`
- `ALTER (DATABASE|SCHEMA) database_name SET DBPROPERTIES (property_name=property_value, ...)`
Another PR will be submitted to handle the unsupported commands. In the Database-related DDL commands, we will issue an error exception for `ALTER (DATABASE|SCHEMA) database_name SET OWNER [USER|ROLE] user_or_role`.
cc @yhuai @andrewor14 @rxin Could you review the changes? Is it in the right direction? Thanks!
#### How was this patch tested?
Added a few test cases in `command/DDLSuite.scala` for testing DDL command execution in `SQLContext`. Since `HiveContext` also shares the same implementation, the existing test cases in `\hive` also verifies the correctness of these commands.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark dbDDL
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12009.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12009
----
commit 602cace43c7b2c2ae3c59b6b742a58cf986d03f8
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-25T17:14:31Z
native DDL support for create database
commit e26542e248796c4db1dc830b4c0febe48b7284f3
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-27T15:34:08Z
Merge remote-tracking branch 'upstream/master' into dbDDL
commit 2794bfa2f3675c5703f98540f3663a4cfe3c9126
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-27T18:28:58Z
set the directory for database creation
commit 65ce660dc68576e43c6c96ba8481643787bafde3
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T02:02:34Z
add a default db extension.
commit aba3a959a5f308958a9924873057ca25222d322e
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T16:52:12Z
native support for drop/alter/describe database
commit ffe0c7aca597c749c6ae755500a440b5466180da
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T18:09:24Z
add more test cases.
commit d7c36488746fb9c414aaf2f28aceeefcd078ba95
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T18:15:26Z
added an extra line.
commit 9420f08408810ee3bce027d1dcbdefa848ae8d13
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T19:25:16Z
added more comments.
commit 4dab82c833e84d0426b550b88945e8821933715f
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T19:30:09Z
update the comment.
commit f4c33e236334057be545d25c3147e4ecbb412a99
Author: gatorsmile <ga...@gmail.com>
Date: 2016-03-28T19:36:26Z
update the comment.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57679159
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala ---
@@ -318,6 +320,138 @@ case class DescribeCommand(
}
/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
+case class CreateDatabase(
--- End diff --
Sure, will move them back. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203170040
LGTM will merge this once tests pass.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57665228
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala ---
@@ -318,6 +320,138 @@ case class DescribeCommand(
}
/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
+case class CreateDatabase(
--- End diff --
Agreed. We should keep them in `ddl.scala`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57679169
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala ---
@@ -318,6 +320,138 @@ case class DescribeCommand(
}
/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
+case class CreateDatabase(
+ databaseName: String,
+ ifNotExists: Boolean,
+ path: Option[String],
+ comment: Option[String],
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
+
+/**
+ * A command for users to remove a database from the system.
+ *
+ * 'ignoreIfNotExists':
+ * - true, if database_name does't exist, no action
+ * - false (default), if database_name does't exist, a warning message will be issued
+ * 'cascade':
+ * - true, the dependent objects are automatically dropped before dropping database.
+ * - false (default), it is in the Restrict mode. The database cannot be dropped if
+ * it is not empty. The inclusive tables must be dropped at first.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * DROP DATABASE [IF EXISTS] database_name [RESTRICT|CASCADE];
+ * }}}
+ */
+case class DropDatabase(
+ databaseName: String,
+ ignoreIfNotExists: Boolean,
--- End diff --
Sure, will do. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r58091330
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
Yea. Let's create the directory if it is not created.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57642492
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala ---
@@ -318,6 +320,138 @@ case class DescribeCommand(
}
/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
+case class CreateDatabase(
+ databaseName: String,
+ ifNotExists: Boolean,
+ path: Option[String],
+ comment: Option[String],
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
+
+/**
+ * A command for users to remove a database from the system.
+ *
+ * 'ignoreIfNotExists':
+ * - true, if database_name does't exist, no action
+ * - false (default), if database_name does't exist, a warning message will be issued
+ * 'cascade':
+ * - true, the dependent objects are automatically dropped before dropping database.
+ * - false (default), it is in the Restrict mode. The database cannot be dropped if
+ * it is not empty. The inclusive tables must be dropped at first.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * DROP DATABASE [IF EXISTS] database_name [RESTRICT|CASCADE];
+ * }}}
+ */
+case class DropDatabase(
+ databaseName: String,
+ ignoreIfNotExists: Boolean,
--- End diff --
let's call this `ifExists` to be consistent
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202562845
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57630288
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -39,7 +39,7 @@ abstract class ExternalCatalog {
protected def requireDbExists(db: String): Unit = {
if (!databaseExists(db)) {
- throw new AnalysisException(s"Database $db does not exist")
+ throw new AnalysisException(s"Database '$db' does not exist")
--- End diff --
Ok. Sure. We need to change all the places.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57624324
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -39,7 +39,7 @@ abstract class ExternalCatalog {
protected def requireDbExists(db: String): Unit = {
if (!databaseExists(db)) {
- throw new AnalysisException(s"Database $db does not exist")
+ throw new AnalysisException(s"Database '$db' does not exist")
--- End diff --
maybe use backtick?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203133823
**[Test build #54468 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54468/consoleFull)** for PR 12009 at commit [`f22ef90`](https://github.com/apache/spark/commit/f22ef9029c20934f6763ed9b52aa2901460c5a84).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203175309
Thank you for your reviews!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57823462
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
+
/**
- * Drop Database: Removes a database from the system.
+ * A command for users to remove a database from the system.
*
* 'ifExists':
* - true, if database_name does't exist, no action
* - false (default), if database_name does't exist, a warning message will be issued
- * 'restric':
- * - true (default), the database cannot be dropped if it is not empty. The inclusive
- * tables must be dropped at first.
- * - false, it is in the Cascade mode. The dependent objects are automatically dropped
- * before dropping database.
+ * 'cascade':
+ * - true, the dependent objects are automatically dropped before dropping database.
+ * - false (default), it is in the Restrict mode. The database cannot be dropped if
+ * it is not empty. The inclusive tables must be dropped at first.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * DROP DATABASE [IF EXISTS] database_name [RESTRICT|CASCADE];
+ * }}}
*/
case class DropDatabase(
databaseName: String,
ifExists: Boolean,
- restrict: Boolean)(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ cascade: Boolean)
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ sqlContext.sessionState.catalog.dropDatabase(databaseName, ifExists, cascade)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
-/** ALTER DATABASE: add new (key, value) pairs into DBPROPERTIES */
+/**
+ * A command for users to add new (key, value) pairs into DBPROPERTIES
+ * If the database does not exist, an error message will be issued to indicate the database
+ * does not exist.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * ALTER (DATABASE|SCHEMA) database_name SET DBPROPERTIES (property_name=property_value, ...)
+ * }}}
+ */
case class AlterDatabaseProperties(
databaseName: String,
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ val db: CatalogDatabase = catalog.getDatabase(databaseName)
+ catalog.alterDatabase(db.copy(properties = db.properties ++ props))
+
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
/**
- * DESCRIBE DATABASE: shows the name of the database, its comment (if one has been set), and its
+ * A command for users to show the name of the database, its comment (if one has been set), and its
* root location on the filesystem. When extended is true, it also shows the database's properties
+ * If the database does not exist, an error message will be issued to indicate the database
+ * does not exist.
+ * The syntax of using this command in SQL is
+ * {{{
+ * DESCRIBE DATABASE [EXTENDED] db_name
+ * }}}
*/
case class DescribeDatabase(
databaseName: String,
- extended: Boolean)(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ extended: Boolean)
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val dbMetadata: CatalogDatabase = sqlContext.sessionState.catalog.getDatabase(databaseName)
+ val result =
+ Row("Database Name", dbMetadata.name) ::
--- End diff --
this can probably just be `Name`. The user ran `DESCRIBE DATABASE` so it's pretty obvious
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202562807
**[Test build #54348 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54348/consoleFull)** for PR 12009 at commit [`f4c33e2`](https://github.com/apache/spark/commit/f4c33e236334057be545d25c3147e4ecbb412a99).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57642335
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -104,6 +104,13 @@ class SessionCatalog(externalCatalog: ExternalCatalog, conf: CatalystConf) {
currentDb = db
}
+ // todo: what is the default path in SessionCatalog?
+ def getDefaultPath: String = ""
--- End diff --
this is only used for the database right? I'd remove this and introduce only `getDefaultDBPath`. For `SessionCatalog` I would just use the temp dir.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203169871
**[Test build #54468 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54468/consoleFull)** for PR 12009 at commit [`f22ef90`](https://github.com/apache/spark/commit/f22ef9029c20934f6763ed9b52aa2901460c5a84).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202555613
**[Test build #54348 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54348/consoleFull)** for PR 12009 at commit [`f4c33e2`](https://github.com/apache/spark/commit/f4c33e236334057be545d25c3147e4ecbb412a99).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57642224
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -104,6 +104,13 @@ class SessionCatalog(externalCatalog: ExternalCatalog, conf: CatalystConf) {
currentDb = db
}
+ // todo: what is the default path in SessionCatalog?
+ def getDefaultPath: String = ""
+
+ def getDefaultDBExtension: String = ".db"
--- End diff --
either way I don't think we want this; just embed it in `getDefaultDBPath`. Otherwise we'll end up with too many random methods
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203161018
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54461/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203116744
**[Test build #54461 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54461/consoleFull)** for PR 12009 at commit [`16c829e`](https://github.com/apache/spark/commit/16c829e6e14e177a1950693604a431cc8a2ed4fc).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203160828
**[Test build #54461 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54461/consoleFull)** for PR 12009 at commit [`16c829e`](https://github.com/apache/spark/commit/16c829e6e14e177a1950693604a431cc8a2ed4fc).
* This patch passes all tests.
* This patch **does not merge cleanly**.
* This patch adds the following public classes _(experimental)_:
* `case class DescribeDatabase(`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57639140
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -0,0 +1,135 @@
+/*
+ * 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.command
+
+import org.apache.spark.sql.{AnalysisException, QueryTest, Row}
+import org.apache.spark.sql.catalyst.catalog.CatalogDatabase
+import org.apache.spark.sql.catalyst.parser.ParserUtils._
+import org.apache.spark.sql.test.SharedSQLContext
+
+class DDLSuite extends QueryTest with SharedSQLContext {
+
+ /**
+ * Drops database `databaseName` after calling `f`.
+ */
+ private def withDatabase(dbNames: String*)(f: => Unit): Unit = {
+ try f finally {
+ dbNames.foreach { name =>
+ sqlContext.sql(s"DROP DATABASE IF EXISTS $name CASCADE")
+ }
+ }
+ }
+
+ test("Create/Drop/Alter/Describe Database - basic") {
--- End diff --
can you split these into separate tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202743732
Sure, will do the changes. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57638935
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -39,7 +39,7 @@ abstract class ExternalCatalog {
protected def requireDbExists(db: String): Unit = {
if (!databaseExists(db)) {
- throw new AnalysisException(s"Database $db does not exist")
+ throw new AnalysisException(s"Database '$db' does not exist")
--- End diff --
let's change it in a separate patch then. There are a few other places like this
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57823588
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
+
/**
- * Drop Database: Removes a database from the system.
+ * A command for users to remove a database from the system.
*
* 'ifExists':
* - true, if database_name does't exist, no action
* - false (default), if database_name does't exist, a warning message will be issued
- * 'restric':
- * - true (default), the database cannot be dropped if it is not empty. The inclusive
- * tables must be dropped at first.
- * - false, it is in the Cascade mode. The dependent objects are automatically dropped
- * before dropping database.
+ * 'cascade':
+ * - true, the dependent objects are automatically dropped before dropping database.
+ * - false (default), it is in the Restrict mode. The database cannot be dropped if
+ * it is not empty. The inclusive tables must be dropped at first.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * DROP DATABASE [IF EXISTS] database_name [RESTRICT|CASCADE];
+ * }}}
*/
case class DropDatabase(
databaseName: String,
ifExists: Boolean,
- restrict: Boolean)(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ cascade: Boolean)
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ sqlContext.sessionState.catalog.dropDatabase(databaseName, ifExists, cascade)
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
-/** ALTER DATABASE: add new (key, value) pairs into DBPROPERTIES */
+/**
+ * A command for users to add new (key, value) pairs into DBPROPERTIES
+ * If the database does not exist, an error message will be issued to indicate the database
+ * does not exist.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * ALTER (DATABASE|SCHEMA) database_name SET DBPROPERTIES (property_name=property_value, ...)
+ * }}}
+ */
case class AlterDatabaseProperties(
databaseName: String,
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ val db: CatalogDatabase = catalog.getDatabase(databaseName)
+ catalog.alterDatabase(db.copy(properties = db.properties ++ props))
+
+ Seq.empty[Row]
+ }
+
+ override val output: Seq[Attribute] = Seq.empty
+}
/**
- * DESCRIBE DATABASE: shows the name of the database, its comment (if one has been set), and its
+ * A command for users to show the name of the database, its comment (if one has been set), and its
* root location on the filesystem. When extended is true, it also shows the database's properties
+ * If the database does not exist, an error message will be issued to indicate the database
+ * does not exist.
+ * The syntax of using this command in SQL is
+ * {{{
+ * DESCRIBE DATABASE [EXTENDED] db_name
+ * }}}
*/
case class DescribeDatabase(
databaseName: String,
- extended: Boolean)(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ extended: Boolean)
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val dbMetadata: CatalogDatabase = sqlContext.sessionState.catalog.getDatabase(databaseName)
+ val result =
+ Row("Database Name", dbMetadata.name) ::
--- End diff --
I'll change this myself
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57641047
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -39,7 +39,7 @@ abstract class ExternalCatalog {
protected def requireDbExists(db: String): Unit = {
if (!databaseExists(db)) {
- throw new AnalysisException(s"Database $db does not exist")
+ throw new AnalysisException(s"Database '$db' does not exist")
--- End diff --
Sure, will do it in a separate PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202601202
Looks great.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r58168784
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
@yhuai I did try it. Actually, the code is done... However, if we create a directory before issuing Hive client API `createDatabase`, we will get the following error message from Hive:
```
Error in query: org.apache.hadoop.hive.metastore.api.AlreadyExistsException: Database db3 already exists;
```
Just feel free to let me know what I should do next. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r58120693
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
I see. Will do it in https://github.com/apache/spark/pull/12081. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57990860
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
True. Only did it in the default DB path. I forgot to do it in the regular case : ( Let me submit a follow-up PR for it. Thanks for catching it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203170065
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203161016
Build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202687564
I just merged https://github.com/apache/spark/pull/12015
Can you update to use the new ANTLR4 parser instead? We are going to remove the ANTLR3 one in the next day or two. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57679194
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -104,6 +104,13 @@ class SessionCatalog(externalCatalog: ExternalCatalog, conf: CatalystConf) {
currentDb = db
}
+ // todo: what is the default path in SessionCatalog?
+ def getDefaultPath: String = ""
--- End diff --
ok, will do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57641072
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -0,0 +1,135 @@
+/*
+ * 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.command
+
+import org.apache.spark.sql.{AnalysisException, QueryTest, Row}
+import org.apache.spark.sql.catalyst.catalog.CatalogDatabase
+import org.apache.spark.sql.catalyst.parser.ParserUtils._
+import org.apache.spark.sql.test.SharedSQLContext
+
+class DDLSuite extends QueryTest with SharedSQLContext {
+
+ /**
+ * Drops database `databaseName` after calling `f`.
+ */
+ private def withDatabase(dbNames: String*)(f: => Unit): Unit = {
+ try f finally {
+ dbNames.foreach { name =>
+ sqlContext.sql(s"DROP DATABASE IF EXISTS $name CASCADE")
+ }
+ }
+ }
+
+ test("Create/Drop/Alter/Describe Database - basic") {
--- End diff --
Sure, will do it. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12009
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r58006853
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
I tried it in `spark-sql`. If the directory is not created, Hive will do it for us. I am wondering if we still should create directory in Spark?
However, this PR has an issue when users specify the location in the `Create Database` command. The generated path should be `path/databaseName.db` instead of `path`. Will fix it soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57979921
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -45,46 +45,135 @@ abstract class NativeDDLCommand(val sql: String) extends RunnableCommand {
}
+/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
case class CreateDatabase(
databaseName: String,
ifNotExists: Boolean,
path: Option[String],
comment: Option[String],
- props: Map[String, String])(sql: String)
- extends NativeDDLCommand(sql) with Logging
+ props: Map[String, String])
+ extends RunnableCommand {
+
+ override def run(sqlContext: SQLContext): Seq[Row] = {
+ val catalog = sqlContext.sessionState.catalog
+ catalog.createDatabase(
+ CatalogDatabase(
+ databaseName,
+ comment.getOrElse(""),
+ path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+ props),
+ ifNotExists)
+ Seq.empty[Row]
+ }
--- End diff --
Do we need to create the underlying dir in this command?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203169540
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-202562851
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54348/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57624245
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -104,6 +104,13 @@ class SessionCatalog(externalCatalog: ExternalCatalog, conf: CatalystConf) {
currentDb = db
}
+ // todo: what is the default path in SessionCatalog?
+ def getDefaultPath: String = ""
+
+ def getDefaultDBExtension: String = ".db"
--- End diff --
what is this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57629952
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -104,6 +104,13 @@ class SessionCatalog(externalCatalog: ExternalCatalog, conf: CatalystConf) {
currentDb = db
}
+ // todo: what is the default path in SessionCatalog?
+ def getDefaultPath: String = ""
+
+ def getDefaultDBExtension: String = ".db"
--- End diff --
That is from Hive. http://www.reedbushey.com/99Programming%20Hive.pdf
> The database directory is created under a top-level directory specified by the property hive.metastore.warehouse.dir, which we discussed in “Local Mode Configuration” on page 24 and “Distributed and Pseudodistributed Mode Configura- tion” on page 26. Assuming you are using the default value for this property, /user/hive/ warehouse, when the financials database is created, Hive will create the directory /user/ hive/warehouse/financials.db. Note the .db extension.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203170273
Oh good timing. Merging into master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57823320
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala ---
@@ -97,7 +97,8 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly
// CREATE DATABASE [IF NOT EXISTS] database_name [COMMENT database_comment]
// [LOCATION path] [WITH DBPROPERTIES (key1=val1, key2=val2, ...)];
- case Token("TOK_CREATEDATABASE", Token(databaseName, Nil) :: args) =>
--- End diff --
changes in this file aren't really necessary since the file will be deleted anyway, but for now it's OK to keep
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12009#issuecomment-203170069
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54468/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] Implement Database-related...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12009#discussion_r57660013
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala ---
@@ -318,6 +320,138 @@ case class DescribeCommand(
}
/**
+ * A command for users to create a new database.
+ *
+ * It will issue an error message when the database with the same name already exists,
+ * unless 'ifNotExists' is true.
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE DATABASE|SCHEMA [IF NOT EXISTS] database_name
+ * }}}
+ */
+case class CreateDatabase(
--- End diff --
also, why did you move them to this file? These are DDLs so they belong to `ddl.scala`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org