[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/12081 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217286617 Alright, this is mainly just test changes now. LGTM merging into master 2.0. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074209 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57830/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074492 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57831/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074490 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074394 **[Test build #57831 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57831/consoleFull)** for PR 12081 at commit [`b190b86`](https://github.com/apache/spark/commit/b190b86d36445056b0d15e903498e765cf1555a9). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074116 **[Test build #57830 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57830/consoleFull)** for PR 12081 at commit [`55cf518`](https://github.com/apache/spark/commit/55cf518ec293eeec275aa8a2ca1f160c46d8eb58). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217074208 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217064204 **[Test build #57831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57831/consoleFull)** for PR 12081 at commit [`b190b86`](https://github.com/apache/spark/commit/b190b86d36445056b0d15e903498e765cf1555a9). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-217063822 **[Test build #57830 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57830/consoleFull)** for PR 12081 at commit [`55cf518`](https://github.com/apache/spark/commit/55cf518ec293eeec275aa8a2ca1f160c46d8eb58). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62143597
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -57,7 +60,7 @@ case class CreateDatabase(
CatalogDatabase(
databaseName,
comment.getOrElse(""),
-path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+catalog.createDatabasePath(databaseName, path),
--- End diff --
Just FYI, `getDefaultDBPath` is just used for the following use case:
```
Get the path for creating a non-default database when database location is
not provided by users.
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62106217
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -57,7 +60,7 @@ case class CreateDatabase(
CatalogDatabase(
databaseName,
comment.getOrElse(""),
-path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+catalog.createDatabasePath(databaseName, path),
--- End diff --
Yeah, it sounds like this issue has been fixed by the other PRs. Will
revert it back.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62105086
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -365,4 +381,113 @@ class HiveDDLSuite extends QueryTest with
SQLTestUtils with TestHiveSingleton {
.exists(_.getString(0) == "# Detailed Table Information"))
}
}
+
+ private def createDatabaseWithLocation(tmpDir: File, dirExists:
Boolean): Unit = {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val tabName = "tab1"
+val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+withTable(tabName) {
+ if (dirExists) {
+assert(tmpDir.listFiles.isEmpty)
+ } else {
+assert(!fs.exists(new Path(tmpDir.toString)))
+ }
+ sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
+ val db1 = catalog.getDatabaseMetadata(dbName)
+ assert(db1 == CatalogDatabase(
+dbName,
+"",
+path,
+Map.empty))
+ sql("USE db1")
+
+ sql(s"CREATE TABLE $tabName as SELECT 1")
+ assert(tableDirectoryExists(TableIdentifier(tabName),
Option(tmpDir.toString)))
+
+ assert(tmpDir.listFiles.nonEmpty)
+ sql(s"DROP TABLE $tabName")
+
+ assert(tmpDir.listFiles.isEmpty)
+ sql(s"DROP DATABASE $dbName")
+ assert(!fs.exists(new Path(tmpDir.toString)))
+}
+ }
+
+ test("create/drop database - location without pre-created directory") {
+ withTempPath { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = false)
+}
+ }
+
+ test("create/drop database - location with pre-created directory") {
+withTempDir { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = true)
+}
+ }
+
+ test("create/drop database - RESTRICT") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = "file:" + catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+val db1 = catalog.getDatabaseMetadata(dbName)
+assert(db1 == CatalogDatabase(
+ dbName,
+ "",
+ path,
+ Map.empty))
+// the database directory was created
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName")
+// the database directory was removed
+assert(!fs.exists(dbPath))
+ }
+
+ test("create/drop database - CASCADE") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName CASCADE")
--- End diff --
Sure will do 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62105072
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -365,4 +381,113 @@ class HiveDDLSuite extends QueryTest with
SQLTestUtils with TestHiveSingleton {
.exists(_.getString(0) == "# Detailed Table Information"))
}
}
+
+ private def createDatabaseWithLocation(tmpDir: File, dirExists:
Boolean): Unit = {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val tabName = "tab1"
+val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+withTable(tabName) {
+ if (dirExists) {
+assert(tmpDir.listFiles.isEmpty)
+ } else {
+assert(!fs.exists(new Path(tmpDir.toString)))
+ }
+ sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
+ val db1 = catalog.getDatabaseMetadata(dbName)
+ assert(db1 == CatalogDatabase(
+dbName,
+"",
+path,
+Map.empty))
+ sql("USE db1")
+
+ sql(s"CREATE TABLE $tabName as SELECT 1")
+ assert(tableDirectoryExists(TableIdentifier(tabName),
Option(tmpDir.toString)))
+
+ assert(tmpDir.listFiles.nonEmpty)
+ sql(s"DROP TABLE $tabName")
+
+ assert(tmpDir.listFiles.isEmpty)
+ sql(s"DROP DATABASE $dbName")
+ assert(!fs.exists(new Path(tmpDir.toString)))
+}
+ }
+
+ test("create/drop database - location without pre-created directory") {
+ withTempPath { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = false)
+}
+ }
+
+ test("create/drop database - location with pre-created directory") {
+withTempDir { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = true)
+}
+ }
+
+ test("create/drop database - RESTRICT") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = "file:" + catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+val db1 = catalog.getDatabaseMetadata(dbName)
+assert(db1 == CatalogDatabase(
+ dbName,
+ "",
+ path,
+ Map.empty))
+// the database directory was created
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName")
+// the database directory was removed
+assert(!fs.exists(dbPath))
+ }
--- End diff --
Sure, will do 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62104915
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -20,21 +20,37 @@ package org.apache.spark.sql.hive.execution
import java.io.File
import org.apache.hadoop.fs.Path
+import org.scalatest.BeforeAndAfterEach
import org.apache.spark.sql.{AnalysisException, QueryTest, SaveMode}
-import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.catalog.{CatalogDatabase,
CatalogTableType}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.hive.test.TestHiveSingleton
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.test.SQLTestUtils
-class HiveDDLSuite extends QueryTest with SQLTestUtils with
TestHiveSingleton {
+class HiveDDLSuite
+ extends QueryTest with SQLTestUtils with TestHiveSingleton with
BeforeAndAfterEach {
import hiveContext.implicits._
+ override def afterEach(): Unit = {
+try {
+ // drop all databases, tables and functions after each test
+ sqlContext.sessionState.catalog.reset()
+} finally {
+ super.afterEach()
+}
+ }
// check if the directory for recording the data of the table exists.
- private def tableDirectoryExists(tableIdentifier: TableIdentifier):
Boolean = {
+ private def tableDirectoryExists(
+ tableIdentifier: TableIdentifier,
+ dbPath: Option[String] = None): Boolean = {
val expectedTablePath =
+if (dbPath.isEmpty) {
hiveContext.sessionState.catalog.hiveDefaultTableFilePath(tableIdentifier)
--- End diff --
Sure, will do 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62104748
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -95,49 +95,85 @@ class DDLSuite extends QueryTest with SharedSQLContext
with BeforeAndAfterEach {
catalog.createPartitions(tableName, Seq(part), ignoreIfExists = false)
}
+ private def dropTable(catalog: SessionCatalog, name: TableIdentifier):
Unit = {
+catalog.dropTable(name, ignoreIfNotExists = false)
+ }
--- End diff --
It is being used by one test case. Yeah, I can move it into the test case,
instead of creating a new function here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62104377
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
--- End diff --
Sure, will do the change.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62103858
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -57,7 +60,7 @@ case class CreateDatabase(
CatalogDatabase(
databaseName,
comment.getOrElse(""),
-path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+catalog.createDatabasePath(databaseName, path),
--- End diff --
yeah, actually how is `new Path(p).toString` different from `p`? Looks like
that is the only functional difference here? If that doesn't do anything then
let's just revert all these changes here and only commit the test changes.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62103066
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
--- End diff --
let's just call this `defaultDatabasePath`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216979776 @gatorsmile Thank you for updating this. Can you address my comments? Then, let's get it in! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62101534
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -365,4 +381,113 @@ class HiveDDLSuite extends QueryTest with
SQLTestUtils with TestHiveSingleton {
.exists(_.getString(0) == "# Detailed Table Information"))
}
}
+
+ private def createDatabaseWithLocation(tmpDir: File, dirExists:
Boolean): Unit = {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val tabName = "tab1"
+val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+withTable(tabName) {
+ if (dirExists) {
+assert(tmpDir.listFiles.isEmpty)
+ } else {
+assert(!fs.exists(new Path(tmpDir.toString)))
+ }
+ sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
+ val db1 = catalog.getDatabaseMetadata(dbName)
+ assert(db1 == CatalogDatabase(
+dbName,
+"",
+path,
+Map.empty))
+ sql("USE db1")
+
+ sql(s"CREATE TABLE $tabName as SELECT 1")
+ assert(tableDirectoryExists(TableIdentifier(tabName),
Option(tmpDir.toString)))
+
+ assert(tmpDir.listFiles.nonEmpty)
+ sql(s"DROP TABLE $tabName")
+
+ assert(tmpDir.listFiles.isEmpty)
+ sql(s"DROP DATABASE $dbName")
+ assert(!fs.exists(new Path(tmpDir.toString)))
+}
+ }
+
+ test("create/drop database - location without pre-created directory") {
+ withTempPath { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = false)
+}
+ }
+
+ test("create/drop database - location with pre-created directory") {
+withTempDir { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = true)
+}
+ }
+
+ test("create/drop database - RESTRICT") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = "file:" + catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+val db1 = catalog.getDatabaseMetadata(dbName)
+assert(db1 == CatalogDatabase(
+ dbName,
+ "",
+ path,
+ Map.empty))
+// the database directory was created
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName")
+// the database directory was removed
+assert(!fs.exists(dbPath))
+ }
+
+ test("create/drop database - CASCADE") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName CASCADE")
--- End diff --
Also test that we can drop the db even it still contains tables?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62101477
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -365,4 +381,113 @@ class HiveDDLSuite extends QueryTest with
SQLTestUtils with TestHiveSingleton {
.exists(_.getString(0) == "# Detailed Table Information"))
}
}
+
+ private def createDatabaseWithLocation(tmpDir: File, dirExists:
Boolean): Unit = {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val tabName = "tab1"
+val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+withTable(tabName) {
+ if (dirExists) {
+assert(tmpDir.listFiles.isEmpty)
+ } else {
+assert(!fs.exists(new Path(tmpDir.toString)))
+ }
+ sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
+ val db1 = catalog.getDatabaseMetadata(dbName)
+ assert(db1 == CatalogDatabase(
+dbName,
+"",
+path,
+Map.empty))
+ sql("USE db1")
+
+ sql(s"CREATE TABLE $tabName as SELECT 1")
+ assert(tableDirectoryExists(TableIdentifier(tabName),
Option(tmpDir.toString)))
+
+ assert(tmpDir.listFiles.nonEmpty)
+ sql(s"DROP TABLE $tabName")
+
+ assert(tmpDir.listFiles.isEmpty)
+ sql(s"DROP DATABASE $dbName")
+ assert(!fs.exists(new Path(tmpDir.toString)))
+}
+ }
+
+ test("create/drop database - location without pre-created directory") {
+ withTempPath { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = false)
+}
+ }
+
+ test("create/drop database - location with pre-created directory") {
+withTempDir { tmpDir =>
+ createDatabaseWithLocation(tmpDir, dirExists = true)
+}
+ }
+
+ test("create/drop database - RESTRICT") {
+val catalog = sqlContext.sessionState.catalog
+val dbName = "db1"
+val path = "file:" + catalog.createDatabasePath(dbName, None)
+val dbPath = new Path(path)
+val fs = dbPath.getFileSystem(hiveContext.sessionState.newHadoopConf())
+// the database directory does not exist
+assert(!fs.exists(dbPath))
+
+sql(s"CREATE DATABASE $dbName")
+val db1 = catalog.getDatabaseMetadata(dbName)
+assert(db1 == CatalogDatabase(
+ dbName,
+ "",
+ path,
+ Map.empty))
+// the database directory was created
+assert(fs.exists(dbPath) && fs.isDirectory(dbPath))
+sql("USE db1")
+
+val tabName = "tab1"
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"CREATE TABLE $tabName as SELECT 1")
+assert(tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+sql(s"DROP TABLE $tabName")
+assert(!tableDirectoryExists(TableIdentifier(tabName), Option(path)))
+
+sql(s"DROP DATABASE $dbName")
+// the database directory was removed
+assert(!fs.exists(dbPath))
+ }
--- End diff --
Also test that in restrict mode, we cannot drop a db if it still has table?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62100995
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -20,21 +20,37 @@ package org.apache.spark.sql.hive.execution
import java.io.File
import org.apache.hadoop.fs.Path
+import org.scalatest.BeforeAndAfterEach
import org.apache.spark.sql.{AnalysisException, QueryTest, SaveMode}
-import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.catalog.{CatalogDatabase,
CatalogTableType}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.hive.test.TestHiveSingleton
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.test.SQLTestUtils
-class HiveDDLSuite extends QueryTest with SQLTestUtils with
TestHiveSingleton {
+class HiveDDLSuite
+ extends QueryTest with SQLTestUtils with TestHiveSingleton with
BeforeAndAfterEach {
import hiveContext.implicits._
+ override def afterEach(): Unit = {
+try {
+ // drop all databases, tables and functions after each test
+ sqlContext.sessionState.catalog.reset()
+} finally {
+ super.afterEach()
+}
+ }
// check if the directory for recording the data of the table exists.
- private def tableDirectoryExists(tableIdentifier: TableIdentifier):
Boolean = {
+ private def tableDirectoryExists(
+ tableIdentifier: TableIdentifier,
+ dbPath: Option[String] = None): Boolean = {
val expectedTablePath =
+if (dbPath.isEmpty) {
hiveContext.sessionState.catalog.hiveDefaultTableFilePath(tableIdentifier)
--- End diff --
While you are at here, can you change this to `defaultTablePath`? I think
hiveDefaultTableFilePath is not actually used and we can remove it in another
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62100913
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -57,7 +60,7 @@ case class CreateDatabase(
CatalogDatabase(
databaseName,
comment.getOrElse(""),
-path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+catalog.createDatabasePath(databaseName, path),
--- End diff --
oh, we have `defaultTablePath` in SessionCatalog. Let's use
`defaultDatabasePath` as the better. What do you think?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62100306
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -95,49 +95,85 @@ class DDLSuite extends QueryTest with SharedSQLContext
with BeforeAndAfterEach {
catalog.createPartitions(tableName, Seq(part), ignoreIfExists = false)
}
+ private def dropTable(catalog: SessionCatalog, name: TableIdentifier):
Unit = {
+catalog.dropTable(name, ignoreIfNotExists = false)
+ }
--- End diff --
Seems we do not need this method?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r62100086
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -57,7 +60,7 @@ case class CreateDatabase(
CatalogDatabase(
databaseName,
comment.getOrElse(""),
-path.getOrElse(catalog.getDefaultDBPath(databaseName)),
+catalog.createDatabasePath(databaseName, path),
--- End diff --
looking at it again. Seems we do not really need to pass in `path`, right?
Also, how about we keep the name getDefulatDBPath because `createDatabasePath`
implied file system operations (sorry). But, it will be good to add comment
to this method to explain its purpose.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216967458 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57772/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216967453 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216967090 **[Test build #57772 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57772/consoleFull)** for PR 12081 at commit [`838eaeb`](https://github.com/apache/spark/commit/838eaeb47e4d129bf0e93351fd5e6e4db25a2ace). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216940066 **[Test build #57772 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57772/consoleFull)** for PR 12081 at commit [`838eaeb`](https://github.com/apache/spark/commit/838eaeb47e4d129bf0e93351fd5e6e4db25a2ace). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216895583 : ) This one needs to be rebased again. 9 times --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216764582 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57722/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216764581 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216764411 **[Test build #57722 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57722/consoleFull)** for PR 12081 at commit [`b885f7b`](https://github.com/apache/spark/commit/b885f7be20a4e6a5355edfea86aefa7b702a56f7). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216761773 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216761778 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57717/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216761612 **[Test build #57717 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57717/consoleFull)** for PR 12081 at commit [`59a9805`](https://github.com/apache/spark/commit/59a9805ceacfc129bb3f8fef1cd2cd7d8d7f7f70). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216752333 @andrewor14 I also think about creating directory in implementation of create database. Metastore APIs created the directory for us no matter whether the provided location/directory exists or not. The following test cases verified it. https://github.com/gatorsmile/spark/blob/mkdir/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala#L401-L411 Should we still create the database directory? If yes, could we do it in a separate PR? This PR has a long change history. It has been rebased 8 times. It is hard for the future readers to follow what we did in this 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216751211 **[Test build #57722 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57722/consoleFull)** for PR 12081 at commit [`b885f7b`](https://github.com/apache/spark/commit/b885f7be20a4e6a5355edfea86aefa7b702a56f7). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61991923
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
--- End diff --
I did try it, but it involves a lot of code changes.
The major issue is the type mismatch between the `path` in `case class
CreateDatabase` and the `locationUri` in `CatalogDatabase`. The first one has
to be `Option[String]` since users might not provide `location` in the create
table command. The second one has to be `String` since we always have the
`location`.
https://github.com/gatorsmile/spark/blob/mkdir/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala#L63
If we change the type of `locationUri` in `CatalogDatabase` to
`Option[String]`, the related code changes look not clean.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216748843 **[Test build #57717 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57717/consoleFull)** for PR 12081 at commit [`59a9805`](https://github.com/apache/spark/commit/59a9805ceacfc129bb3f8fef1cd2cd7d8d7f7f70). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61991201
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -348,4 +364,99 @@ class HiveDDLSuite extends QueryTest with SQLTestUtils
with TestHiveSingleton {
}
}
}
+
+ test("create/drop database - location") {
+val catalog = sqlContext.sessionState.catalog
+withTempDir { tmpDir =>
+ val dbName = "db1"
+ val tabName = "tab1"
+ val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+ val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+ withTable(tabName) {
+assert(tmpDir.listFiles.isEmpty)
+sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
--- End diff --
The directory will be dropped by the Hive metastore API. The test case
verified this behavior.
https://github.com/gatorsmile/spark/blob/mkdir/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala#L394
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61988141
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
---
@@ -348,4 +364,99 @@ class HiveDDLSuite extends QueryTest with SQLTestUtils
with TestHiveSingleton {
}
}
}
+
+ test("create/drop database - location") {
+val catalog = sqlContext.sessionState.catalog
+withTempDir { tmpDir =>
+ val dbName = "db1"
+ val tabName = "tab1"
+ val path = "file:" + catalog.createDatabasePath(dbName,
Option(tmpDir.toString))
+ val fs = new
Path(path).getFileSystem(hiveContext.sessionState.newHadoopConf())
+ withTable(tabName) {
+assert(tmpDir.listFiles.isEmpty)
+sql(s"CREATE DATABASE $dbName Location '$tmpDir'")
--- End diff --
If we create a database with a given location, then we drop this database,
will we still keep the dir like external table?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61983213
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
--- End diff --
Sure, 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216719267 @andrewor14 You know, this PR has many changes since its initial version. Originally, the focus of this PR is to create a correct warehouse directory. Later, a few related PRs were merged. Thus, I removed the contents. : ) Yeah, sure. I can clean it and try to create the 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61983027
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
val database = if (conf.caseSensitiveAnalysis) db else db.toLowerCase
-new Path(new Path(conf.warehousePath), database + ".db").toString
+val dbPath = path.map(new Path(_)).getOrElse {
+ val defaultPath = conf.warehousePath
+ if (org.apache.commons.lang.StringUtils.isBlank(defaultPath)) {
+throw new AnalysisException("spark.sql.warehouse.dir is blank")
--- End diff --
This is from what Hive metastore is doing.
https://github.com/apache/hive/blob/134b6cccbd7237901f7f7594626796863ca0150a/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java#L74-L77
I also have the same question like you. Let me remove it since we do not
follow what Hive is doing.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61982747
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
val database = if (conf.caseSensitiveAnalysis) db else db.toLowerCase
-new Path(new Path(conf.warehousePath), database + ".db").toString
+val dbPath = path.map(new Path(_)).getOrElse {
+ val defaultPath = conf.warehousePath
+ if (org.apache.commons.lang.StringUtils.isBlank(defaultPath)) {
+throw new AnalysisException("spark.sql.warehouse.dir is blank")
+ }
+ new Path(new Path(defaultPath), database + ".db")
+}
+dbPath.toString.stripSuffix(File.separator)
--- End diff --
This is just to make it consistent with what Hive is doing now. I can
remove it since I found we are not following what Hive is doing now.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216692119 @gatorsmile I took a quick look and I think the logic can be simpler. There are a lot of random checks that don't seem necessary to me. Actually I thought the original goal was to create the dir, but this patch doesn't seem to do that. I'm not sure what the non-test changes here achieve. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61971480
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
val database = if (conf.caseSensitiveAnalysis) db else db.toLowerCase
-new Path(new Path(conf.warehousePath), database + ".db").toString
+val dbPath = path.map(new Path(_)).getOrElse {
+ val defaultPath = conf.warehousePath
+ if (org.apache.commons.lang.StringUtils.isBlank(defaultPath)) {
+throw new AnalysisException("spark.sql.warehouse.dir is blank")
+ }
+ new Path(new Path(defaultPath), database + ".db")
+}
+dbPath.toString.stripSuffix(File.separator)
--- End diff --
similarly I don't understand why we need to do this. What is the exception
you see if we didn't strip the slash?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61971054
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
val database = if (conf.caseSensitiveAnalysis) db else db.toLowerCase
-new Path(new Path(conf.warehousePath), database + ".db").toString
+val dbPath = path.map(new Path(_)).getOrElse {
+ val defaultPath = conf.warehousePath
+ if (org.apache.commons.lang.StringUtils.isBlank(defaultPath)) {
+throw new AnalysisException("spark.sql.warehouse.dir is blank")
--- End diff --
this check is so arbitrary... why do we need 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61970888
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -148,9 +148,17 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
+ /** Get the path for creating a non-default database. */
+ def createDatabasePath(db: String, path: Option[String]): String = {
--- End diff --
actually this doesn't seem to belong to the session catalog. It's only used
in 1 place so I would just move it to `CreateDatabase` itself.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216585702 @cloud-fan Your PR https://github.com/apache/spark/pull/12871 is related to this. Could you please review this? 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216099984 cc @liancheng @yhuai It is ready for review. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216051049 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57480/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216051048 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216050996 **[Test build #57480 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57480/consoleFull)** for PR 12081 at commit [`95d9a00`](https://github.com/apache/spark/commit/95d9a00d0679b932bc6e2478b49c6f8b189054f5). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-216044152 **[Test build #57480 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57480/consoleFull)** for PR 12081 at commit [`95d9a00`](https://github.com/apache/spark/commit/95d9a00d0679b932bc6e2478b49c6f8b189054f5). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user liancheng commented on a diff in the pull request: https://github.com/apache/spark/pull/12081#discussion_r61283810 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala --- @@ -693,7 +697,6 @@ class SessionCatalog( dropDatabase(db, ignoreIfNotExists = false, cascade = true) } tempTables.clear() -functionRegistry.clear() --- End diff -- Is this related to the `current_database()` issue you and @yhuai discussed previously? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61284231
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -118,7 +148,7 @@ class DDLSuite extends QueryTest with SharedSQLContext
with BeforeAndAfterEach {
assert(db1 == CatalogDatabase(
dbNameWithoutBackTicks,
"",
- System.getProperty("java.io.tmpdir") + File.separator +
s"$dbNameWithoutBackTicks.db",
+ appendTrailingSlash(System.getProperty("java.io.tmpdir")) +
s"$dbNameWithoutBackTicks.db",
--- End diff --
Updated the PR description. What Reynold is doing in another PR
https://github.com/apache/spark/pull/12722, this becomes a useless issue. I
will remove the related code changes later.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/12081#discussion_r61284167 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala --- @@ -693,7 +697,6 @@ class SessionCatalog( dropDatabase(db, ignoreIfNotExists = false, cascade = true) } tempTables.clear() -functionRegistry.clear() --- End diff -- Yeah, but now, this is fixed. We do not need the code changes. : ) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61283076
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -118,7 +148,7 @@ class DDLSuite extends QueryTest with SharedSQLContext
with BeforeAndAfterEach {
assert(db1 == CatalogDatabase(
dbNameWithoutBackTicks,
"",
- System.getProperty("java.io.tmpdir") + File.separator +
s"$dbNameWithoutBackTicks.db",
+ appendTrailingSlash(System.getProperty("java.io.tmpdir")) +
s"$dbNameWithoutBackTicks.db",
--- End diff --
This is different from what Hive behaves. Hive always removes the last
`File.separator`, if existing.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r61282206
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -118,7 +148,7 @@ class DDLSuite extends QueryTest with SharedSQLContext
with BeforeAndAfterEach {
assert(db1 == CatalogDatabase(
dbNameWithoutBackTicks,
"",
- System.getProperty("java.io.tmpdir") + File.separator +
s"$dbNameWithoutBackTicks.db",
+ appendTrailingSlash(System.getProperty("java.io.tmpdir")) +
s"$dbNameWithoutBackTicks.db",
--- End diff --
According to the PR description, we should append `File.separator` rather
than slash, right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user liancheng commented on a diff in the pull request: https://github.com/apache/spark/pull/12081#discussion_r61278961 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala --- @@ -693,7 +697,6 @@ class SessionCatalog( dropDatabase(db, ignoreIfNotExists = false, cascade = true) } tempTables.clear() -functionRegistry.clear() --- End diff -- Why do we remove 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/12081#discussion_r61279827 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala --- @@ -693,7 +697,6 @@ class SessionCatalog( dropDatabase(db, ignoreIfNotExists = false, cascade = true) } tempTables.clear() -functionRegistry.clear() --- End diff -- A long story to explain... : ) I need to add it back in the following fix: https://github.com/apache/spark/pull/12081/commits/a2d71fdb1af71d67b744b2f89a08d12837559c9b You are right, I need to add it back. : ) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214942151 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57059/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214942149 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214941942 **[Test build #57059 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57059/consoleFull)** for PR 12081 at commit [`1da261a`](https://github.com/apache/spark/commit/1da261a395bd799b6942e04fa142372a5bee3a8c). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214926445 **[Test build #57059 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57059/consoleFull)** for PR 12081 at commit [`1da261a`](https://github.com/apache/spark/commit/1da261a395bd799b6942e04fa142372a5bee3a8c). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214890715 Many test cases called `SQLConf.clear()` and cleaned all the configurations. When we reloading the conf, we did not load the Hive-related configurations. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214823253 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57004/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214823248 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214823028 **[Test build #57004 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57004/consoleFull)** for PR 12081 at commit [`375b692`](https://github.com/apache/spark/commit/375b69258be2b9b632f681964258b7f963babb4c). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214802017 **[Test build #57004 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57004/consoleFull)** for PR 12081 at commit [`375b692`](https://github.com/apache/spark/commit/375b69258be2b9b632f681964258b7f963babb4c). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214661567 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56980/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214661565 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214661418 **[Test build #56980 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56980/consoleFull)** for PR 12081 at commit [`a2d71fd`](https://github.com/apache/spark/commit/a2d71fdb1af71d67b744b2f89a08d12837559c9b). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214638243 @rxin Thanks for the review! So far, `DDLSuite` only contains the test cases for `InMemoryCatalog`. To verify the behaviors of `HiveExternalCatalog`, we add the test cases in `HiveDDLSuite`. Feel free to let me know if the latest changes resolve all your questions. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214637798 **[Test build #56980 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56980/consoleFull)** for PR 12081 at commit [`a2d71fd`](https://github.com/apache/spark/commit/a2d71fdb1af71d67b744b2f89a08d12837559c9b). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60939762
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -121,8 +123,11 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
-System.getProperty("java.io.tmpdir") + File.separator + db + ".db"
+ def createDatabasePath(dbName: String, path: Option[String]): String = {
--- End diff --
Yeah. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60939806
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -650,7 +655,12 @@ class SessionCatalog(
dropDatabase(db, ignoreIfNotExists = false, cascade = true)
}
tempTables.clear()
-functionRegistry.clear()
+// Do not remove the function `current_database`, which is registered
in each
+// new session of HiveContext. Otherwise, it could load the Hive UDF
function
+// with the same function name.
+functionRegistry.listFunction().filter(_ !=
"current_database").foreach { f =>
--- End diff --
Will make a try.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60939726
--- Diff:
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala ---
@@ -75,9 +76,22 @@ private[sql] class HiveSessionCatalog(
// | Methods and fields for interacting with HiveMetastoreCatalog |
//
- override def getDefaultDBPath(db: String): String = {
-val defaultPath = hiveconf.getVar(HiveConf.ConfVars.METASTOREWAREHOUSE)
-new Path(new Path(defaultPath), db + ".db").toString
+ // This function is to get the path for creating a non-default database
+ override def createDatabasePath(dbName: String, path: Option[String] =
None): String = {
--- End diff --
I see, 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60866200
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -650,7 +655,12 @@ class SessionCatalog(
dropDatabase(db, ignoreIfNotExists = false, cascade = true)
}
tempTables.clear()
-functionRegistry.clear()
+// Do not remove the function `current_database`, which is registered
in each
+// new session of HiveContext. Otherwise, it could load the Hive UDF
function
+// with the same function name.
+functionRegistry.listFunction().filter(_ !=
"current_database").foreach { f =>
--- End diff --
you don't need this anymore, do you?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214139613 Also we should NOT depend on Hive config values. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60863754
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
---
@@ -121,8 +123,11 @@ class SessionCatalog(
currentDb = db
}
- def getDefaultDBPath(db: String): String = {
-System.getProperty("java.io.tmpdir") + File.separator + db + ".db"
+ def createDatabasePath(dbName: String, path: Option[String]): String = {
--- End diff --
Can you define what this function does? Most other functions they are
obvious. It is not clear what this is doing. Is this one creating some new path?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12081#discussion_r60863712
--- Diff:
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala ---
@@ -75,9 +76,22 @@ private[sql] class HiveSessionCatalog(
// | Methods and fields for interacting with HiveMetastoreCatalog |
//
- override def getDefaultDBPath(db: String): String = {
-val defaultPath = hiveconf.getVar(HiveConf.ConfVars.METASTOREWAREHOUSE)
-new Path(new Path(defaultPath), db + ".db").toString
+ // This function is to get the path for creating a non-default database
+ override def createDatabasePath(dbName: String, path: Option[String] =
None): String = {
--- End diff --
we shouldn't use the hive confs here. just use the sqlconf and create a
default warehouse folder config option.
If anything, we should just pre-populate the default warehouse folder
option with the value loaded from hive-site.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214138038 @gatorsmile the new DDLs shouldn't go to the Hive package. We should put all the DDLs in the sql/core package. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214130068 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214130069 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56872/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214129992 **[Test build #56872 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56872/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214104242 **[Test build #56872 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56872/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-214103737 retest this please --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213257176 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56643/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213257173 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213257067 **[Test build #56643 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56643/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213229969 **[Test build #56643 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56643/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user gatorsmile commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213228654 retest this please --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213224445 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56631/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213224442 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213224364 **[Test build #56631 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56631/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-14124] [SQL] [FOLLOWUP] Implement Datab...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12081#issuecomment-213208107 **[Test build #56631 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56631/consoleFull)** for PR 12081 at commit [`44fdf83`](https://github.com/apache/spark/commit/44fdf83c9f519a3671c3db3aae404168e7613be6). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
