You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sandeep-katta <gi...@git.apache.org> on 2018/09/19 11:47:03 UTC
[GitHub] spark pull request #22466: [SPARK-25464][SQL]When database is dropped all th...
GitHub user sandeep-katta opened a pull request:
https://github.com/apache/spark/pull/22466
[SPARK-25464][SQL]When database is dropped all the data related to it is deleted
Modification content:If the database is external then not required to delete it's content.
What changes were proposed in this pull request?
If the user specifies the location while creating the Database then it will be considered as External Database.
For External Database on dropping the database,location will not be deleted.
for e.g create database db1 location '/user/hive/warehouse';
drop database db1;
warehouse folder will not be deleted
How was this patch tested?
Added testcases and also manually verified in the cluster
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sandeep-katta/spark dropDatabse
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22466.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22466
----
commit c2eec35d3fb4c0402c4a243c389316aecb639f6f
Author: sandeep-katta <sa...@...>
Date: 2018-09-18T19:15:52Z
RootCause:When database is dropped all the data related to it is deleted
Modification content:If the database is external then not required to delete it's content.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
yea, in Spark we conflate the two and treat a table as external if location is specified.
However, Hive doesn't have external database, see: https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-Create/Drop/Alter/UseDatabase
I don't want to introduce unnecessary behavior difference from hive, and I feel it's not very useful to have external database. Although your table files can be in an existing folder, but LIST TABLES will not work.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
> That link says Hive does support EXTERNAL. What am I missing?
Hive supports `EXTERNAL` only for tables, not databases.
The CREATE TABLE syntax:
```
CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name ...
```
The CREATE DATABASE syntax:
```
CREATE (DATABASE|SCHEMA) [IF NOT EXISTS] database_name ...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97548/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223389089
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,16 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (fs.exists(dbPath)) {
+ val fileStatus = fs.listStatus(dbPath)
+ if (fileStatus.nonEmpty) {
+ throw new AnalysisException(
+ s"Cannot create Database to the location which is not empty.Given path is ${dbPath}")
--- End diff --
The message here has a few minor typos. Let's try: `s"Cannot create database at location $dbPath because it already exists."`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]When database is dropped all the data ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97434/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #4364 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4364/testReport)** for PR 22466 at commit [`b837d48`](https://github.com/apache/spark/commit/b837d48f688a4fa5aa8766aa076855eb3a6bb97d).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536585
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,17 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25464 create Database with non empty location") {
+ val dbName = "dbwithcustomlocation"
+ withTempDir { tmpDir =>
+ val parentDir = tmpDir.getParent
+ val expectedMsg = s"Cannot create database at location $parentDir because the path is not " +
+ s"empty."
--- End diff --
leading `s` can be removed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22466
The major comments are in the test cases. Could you help clean up the existing test cases?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97614/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96766 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96766/testReport)** for PR 22466 at commit [`86e4d50`](https://github.com/apache/spark/commit/86e4d50d27ecec914ef534e84dcc75965456a32b).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96689 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96689/testReport)** for PR 22466 at commit [`943b7fd`](https://github.com/apache/spark/commit/943b7fddcbb52687af821aa6ca176e74af512ce4).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22466
There is ... see https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-ManagedandExternalTables
I think Spark conflates the two. It's rare (?) but possible to specify a custom location of a managed table, but, typically occurs for `EXTERNAL` tables. So maybe this is OK.
```
private def createTable(tableIdent: TableIdentifier): Unit = {
val storage = DataSource.buildStorageFormatFromOptions(extraOptions.toMap)
val tableType = if (storage.locationUri.isDefined) {
CatalogTableType.EXTERNAL
} else {
CatalogTableType.MANAGED
}
```
And in `SqlParser`:
```
// If location is defined, we'll assume this is an external table.
// Otherwise, we may accidentally delete existing data.
val tableType = if (external || location.isDefined) {
CatalogTableType.EXTERNAL
} else {
CatalogTableType.MANAGED
}
```
So if `LOCATION` implies `EXTERNAL` in Spark, then I get this. `EXTERNAL` tables shouldn't be deleted.
I agree that the Hive impl doesn't seem to take this into account, on the code paths that call `dropDatabase`. CC @andrewor14 in case he is available to comment on the original implementaiton. WDYT @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96929 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96929/testReport)** for PR 22466 at commit [`7577a5a`](https://github.com/apache/spark/commit/7577a5aecd92368cc5ac1b7035d3a1a571dea3fa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
This is a behavior change and makes us different from Hive. However I can't find a strong reason to do it. It's like importing a database, but we can't automatically create table entries in the metastore when creating a database with an existing location.
To me a more reasonable behavior is, fail earlier when creating a database with an existing and non-empty location.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97602/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223395695
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,16 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (fs.exists(dbPath)) {
+ val fileStatus = fs.listStatus(dbPath)
+ if (fileStatus.nonEmpty) {
+ throw new AnalysisException(
+ s"Cannot create Database to the location which is not empty.Given path is ${dbPath}")
--- End diff --
That's fine, sure.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #4364 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4364/testReport)** for PR 22466 at commit [`b837d48`](https://github.com/apache/spark/commit/b837d48f688a4fa5aa8766aa076855eb3a6bb97d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96730/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97544/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96619/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #4381 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4381/testReport)** for PR 22466 at commit [`d290998`](https://github.com/apache/spark/commit/d2909983c26586f078933ef499388ff4189f037c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r225396925
--- Diff: python/pyspark/sql/tests.py ---
@@ -2993,6 +2990,7 @@ def test_current_database(self):
AnalysisException,
"does_not_exist",
lambda: spark.catalog.setCurrentDatabase("does_not_exist"))
+ spark.sql("DROP DATABASE some_db")
--- End diff --
create and drop should be part of test case,if there is any exception then test case will fail.So no need to put in the finally block
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223569270
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -407,6 +407,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
test("create a managed table with the existing non-empty directory") {
withTable("tab1") {
+ Utils.createDirectory(spark.sessionState.conf.warehousePath)
--- End diff --
If we do not delete the directory for each test case, this line is not needed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97602 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97602/testReport)** for PR 22466 at commit [`28a086d`](https://github.com/apache/spark/commit/28a086df2784adc52f76d097a7ee9b724e1b1023).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97589/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #4381 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4381/testReport)** for PR 22466 at commit [`d290998`](https://github.com/apache/spark/commit/d2909983c26586f078933ef499388ff4189f037c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22466
Owp, I've been misreading that several times. Right. Well by analogy, if a database has a non default LOCATION then so do it's tables, and they are treated like EXTERNAL tables. Dropping the DB means dropping the tables, and dropping those tables doesn't delete data. So should the same happen for DBs? Seems sensible, because the DB directory might not even be empty.
Still I feel like I'm missing something if it only comes up in the case that two DBs have the same location, which is going to cause a bunch of other problems.
But is it the right change simply because it's consistent with how dropping tables works?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97589 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97589/testReport)** for PR 22466 at commit [`d862591`](https://github.com/apache/spark/commit/d862591c20ef1d1536d069dcc4f3220ae232c702).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97434 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97434/testReport)** for PR 22466 at commit [`d290998`](https://github.com/apache/spark/commit/d2909983c26586f078933ef499388ff4189f037c).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96750 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96750/testReport)** for PR 22466 at commit [`63dc53a`](https://github.com/apache/spark/commit/63dc53aba545fecfe5301c7ea0b8305e60e20496).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96736/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223569182
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -53,6 +53,7 @@ class HiveCatalogedDDLSuite extends DDLSuite with TestHiveSingleton with BeforeA
// drop all databases, tables and functions after each test
spark.sessionState.catalog.reset()
} finally {
+ Utils.deleteRecursively(new File(spark.sessionState.conf.warehousePath))
--- End diff --
We dropped all the database in line 54, right? Could you fix the test cases instead of delete the directory here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
> See JIRA, I don't think this should be merged.
I have referred Databricks doc https://docs.databricks.com/spark/latest/spark-sql/language-manual/create-database.html and implemented accordingly.Let me know if any suggesstion
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97158/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96766/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
> Btw, what if `create database if not exists ...`? Seems like an exception will be thrown if the table exists even if we specify `if not exists`?
Good catch :+1: ,I have updated the code accordingly
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96689 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96689/testReport)** for PR 22466 at commit [`943b7fd`](https://github.com/apache/spark/commit/943b7fddcbb52687af821aa6ca176e74af512ce4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22466
@sandeep-katta Can you update the PR title?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97182 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97182/testReport)** for PR 22466 at commit [`5d95a09`](https://github.com/apache/spark/commit/5d95a09122436217a8a6e891cfb96babfe49fb2b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97614 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97614/testReport)** for PR 22466 at commit [`b091996`](https://github.com/apache/spark/commit/b09199614c8734c97baee077406d76fde3cb6f02).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97615/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r219440831
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -67,12 +67,15 @@ case class CreateDatabaseCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
+ // if the path is specified by the user then need to consider DB as external DB
--- End diff --
nit: If the location is specified by the user then need to consider DB as external DB,
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
Yes I agree 2 database should not point to same path,**currently this is the loop hole in spark which is required to fix**.If this solution is not okay ,then we can append the dbname.db to the location given by the user
for e.g
create database db1 location /user/hive/warehouse
then the location of the DB should be /user/hive/warehouse/db1.db
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97550 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97550/testReport)** for PR 22466 at commit [`d862591`](https://github.com/apache/spark/commit/d862591c20ef1d1536d069dcc4f3220ae232c702).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96736 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96736/testReport)** for PR 22466 at commit [`f05d7c1`](https://github.com/apache/spark/commit/f05d7c14f3942743485c8b78ac40ee7e1d3bafa9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223394163
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,16 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (fs.exists(dbPath)) {
+ val fileStatus = fs.listStatus(dbPath)
+ if (fileStatus.nonEmpty) {
+ throw new AnalysisException(
+ s"Cannot create Database to the location which is not empty.Given path is ${dbPath}")
--- End diff --
if the folder exists and it is non-empty then we allow to create database,so this statement does not conflict the user ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96730 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96730/testReport)** for PR 22466 at commit [`7d28da0`](https://github.com/apache/spark/commit/7d28da044ca80cd2c06c7d089786bec2657d175f).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536735
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,14 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (!externalCatalog.databaseExists(dbName) && fs.exists(dbPath)
+ && fs.listStatus(dbPath).nonEmpty) {
--- End diff --
Should we necessarily list up files? it's potentially expensive.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97169 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97169/testReport)** for PR 22466 at commit [`5d95a09`](https://github.com/apache/spark/commit/5d95a09122436217a8a6e891cfb96babfe49fb2b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r236005686
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,17 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25464 create a database with a non empty location") {
--- End diff --
Do we have a test case to check "create a database with an empty location"?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226236226
--- Diff: python/pyspark/sql/tests.py ---
@@ -2993,6 +2990,7 @@ def test_current_database(self):
AnalysisException,
"does_not_exist",
lambda: spark.catalog.setCurrentDatabase("does_not_exist"))
+ spark.sql("DROP DATABASE some_db")
--- End diff --
We still need to cleanup each test. Otherwise, if the test fails before dropping `some_db`, it will cause to fail other tests.
Seems like the current cleanup is not good. Let me refine.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]When database is dropped all the data ...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on the issue:
https://github.com/apache/spark/pull/22466
@sandeep-katta can you please update the tile, i think as per your description it seems to be the data will be dropped if the location of the newly created database points to some existing database location.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96730 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96730/testReport)** for PR 22466 at commit [`7d28da0`](https://github.com/apache/spark/commit/7d28da044ca80cd2c06c7d089786bec2657d175f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220873051
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
> Please try the following cases:
>
> * the directory does not exist and the parent directory does not exist too.
spark-sql> create database noparent location '/user/dropTest/noparent/dblocation';
Time taken: 2.246 seconds
> * the directory exists but empty
spark-sql> create database emptydir location '/user/dropTest/emptydir';
Time taken: 0.262 seconds
> * the directory exists and non-empty
spark-sql> create database dirExistsAndNotempty location '/user/dropTest/';
Error in query: Cannot create Database to the location which is not empty.;
> * the directory is not accessible
spark-sql> create database userNoPermission location '/user/hive/dropTest';
Error in query: org.apache.hadoop.hive.ql.metadata.HiveException: MetaException(message:java.security.AccessControlException: Permission denied: user=sandeep, access=WRITE, inode="/user/hive":hive:hive:drwxr-x---
at org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.check(FSPermissionChecker.java:399)
at org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:261)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536773
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -840,12 +840,19 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
}
test("create table in default db") {
- val catalog = spark.sessionState.catalog
- val tableIdent1 = TableIdentifier("tab1", None)
- createTable(catalog, tableIdent1)
- val expectedTableIdent = tableIdent1.copy(database = Some("default"))
- val expectedTable = generateTable(catalog, expectedTableIdent)
- checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ var tablePath: Option[URI] = None
--- End diff --
`var tablePath: URI = null`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97548 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97548/testReport)** for PR 22466 at commit [`675ab8c`](https://github.com/apache/spark/commit/675ab8c60f58f0326b5a98d7205028220d64d4f4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
I am running the same test case with hive version **1.2.1.spark2** and it is passing,can I know with what hive version CI is running and how org.apache.hive.jdbc.HiveStatement and external catalog are linked,I don't see any such code. cc @cloud-fan @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226245229
--- Diff: python/pyspark/sql/tests.py ---
@@ -2993,6 +2990,7 @@ def test_current_database(self):
AnalysisException,
"does_not_exist",
lambda: spark.catalog.setCurrentDatabase("does_not_exist"))
+ spark.sql("DROP DATABASE some_db")
--- End diff --
Submitted #22762.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r222891761
--- Diff: python/pyspark/sql/tests.py ---
@@ -351,7 +351,7 @@ def tearDown(self):
super(SQLTests, self).tearDown()
# tear down test_bucketed_write state
- self.spark.sql("DROP TABLE IF EXISTS pyspark_bucket")
+ self.spark.sql("DROP DATABASE IF EXISTS some_db CASCADE")
--- End diff --
test_current_database,test_list_databases,test_list_tables,test_list_functions and test_list_columns all these test case create the database and does not drop it,so the folder will be present which result in the test case failures
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96601 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96601/testReport)** for PR 22466 at commit [`597f8e6`](https://github.com/apache/spark/commit/597f8e6130965ffbfbdd94bfb4ce3e9c90eed794).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96768/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226548140
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,14 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (!externalCatalog.databaseExists(dbName) && fs.exists(dbPath)
+ && fs.listStatus(dbPath).nonEmpty) {
--- End diff --
listing files is expensive, but create database command is not frequent. Same is mentioned here
https://github.com/apache/spark/pull/22466#discussion_r220795973
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97118 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97118/testReport)** for PR 22466 at commit [`c50e99b`](https://github.com/apache/spark/commit/c50e99b03bcda23762817aeb763971082da8989e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97589 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97589/testReport)** for PR 22466 at commit [`d862591`](https://github.com/apache/spark/commit/d862591c20ef1d1536d069dcc4f3220ae232c702).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96768 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96768/testReport)** for PR 22466 at commit [`3c2831b`](https://github.com/apache/spark/commit/3c2831bf70d4ed83414625fd3e4b7607624d09fc).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22466
That link says Hive does support EXTERNAL. What am I missing? Well, in any event we aren't contemplating a behavior change here.
If you delete a table with LOCATION specified, what should happen? Hive would delete it I guess... unless it's EXTERNAL. Looks like Spark would delete it even when it's in an 'external' location.
If these are conflated I guess we weigh the surprise in not deleting 'local' locations for data when a table is dropped, vs surprise in deleting 'external' locations for data.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
seems @cloud-fan comments are valid as it will not result in any behavior change, I will update the PR accordingly WDYT @srowen
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97434 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97434/testReport)** for PR 22466 at commit [`d290998`](https://github.com/apache/spark/commit/d2909983c26586f078933ef499388ff4189f037c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on the issue:
https://github.com/apache/spark/pull/22466
@srowen @HyukjinKwon I think this can be a risk if the location of the newly created database points to an existing one, if user drop the db both the db data will be lost .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97602 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97602/testReport)** for PR 22466 at commit [`28a086d`](https://github.com/apache/spark/commit/28a086df2784adc52f76d097a7ee9b724e1b1023).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97544 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97544/testReport)** for PR 22466 at commit [`619c396`](https://github.com/apache/spark/commit/619c396a17b2d25f32275ab8ad1dcd113265e218).
* This patch **fails PySpark unit tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22466
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223568531
--- Diff: python/pyspark/sql/tests.py ---
@@ -351,7 +351,7 @@ def tearDown(self):
super(SQLTests, self).tearDown()
# tear down test_bucketed_write state
- self.spark.sql("DROP TABLE IF EXISTS pyspark_bucket")
+ self.spark.sql("DROP DATABASE IF EXISTS some_db CASCADE")
--- End diff --
Could you update the corresponding test cases instead of dropping the database here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97544 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97544/testReport)** for PR 22466 at commit [`619c396`](https://github.com/apache/spark/commit/619c396a17b2d25f32275ab8ad1dcd113265e218).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226673513
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,14 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (!externalCatalog.databaseExists(dbName) && fs.exists(dbPath)
+ && fs.listStatus(dbPath).nonEmpty) {
--- End diff --
as per my knowledge this the only way to check for empty condition, and also I have moved this logic to HiveExternalCatalog
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
you can type the same thing, the jenkins will work for you :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r224666263
--- Diff: python/pyspark/sql/tests.py ---
@@ -350,9 +350,6 @@ def test_sqlcontext_reuses_sparksession(self):
def tearDown(self):
--- End diff --
Now we can remove this method?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96766 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96766/testReport)** for PR 22466 at commit [`86e4d50`](https://github.com/apache/spark/commit/86e4d50d27ecec914ef534e84dcc75965456a32b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
> The major comments are in the test cases. Could you help clean up the existing test cases?
All the comments are fixed and corrected the testcases
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536626
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -840,12 +840,19 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
}
test("create table in default db") {
- val catalog = spark.sessionState.catalog
- val tableIdent1 = TableIdentifier("tab1", None)
- createTable(catalog, tableIdent1)
- val expectedTableIdent = tableIdent1.copy(database = Some("default"))
- val expectedTable = generateTable(catalog, expectedTableIdent)
- checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ var tablePath: Option[URI] = None
+ try {
+ val catalog = spark.sessionState.catalog
+ val tableIdent1 = TableIdentifier("tab1", None)
+ createTable(catalog, tableIdent1)
+ val expectedTableIdent = tableIdent1.copy(database = Some("default"))
+ val expectedTable = generateTable(catalog, expectedTableIdent)
+ tablePath = Some(expectedTable.location)
+ checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ } finally {
+ // This is external table,so it is required to deleted the path
--- End diff --
@HyukjinKwon The first one is `e,s` -> `e, s` ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96750/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96619 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96619/testReport)** for PR 22466 at commit [`597f8e6`](https://github.com/apache/spark/commit/597f8e6130965ffbfbdd94bfb4ce3e9c90eed794).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536555
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,17 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25464 create Database with non empty location") {
--- End diff --
`create a database with a non-empty location`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97182/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220872507
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
> Just want to confirm it what is the behavior of Hive (3.x) when we try to create a database in a non-empty file path?
User can create the Database on the non-empty file path,results are below
host1:/opt/tools/hadoop3.1/hadoop-3.1.1/bin # ./hadoop fs -ls /user/hive/
18/09/27 18:05:39 WARN util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Found 2 items
drwxr-xr-x - root supergroup 0 2018-09-27 18:05 /user/hive/dropTest
drwxr-xr-x - root supergroup 0 2018-09-27 17:19 /user/hive/warehouse
hive> create database testjira location '/user/hive/';
OK
Time taken: 0.187 seconds
hive> desc database testjira;
OK
testjira hdfs://localhost:9000/user/hive root USER
Time taken: 0.045 seconds, Fetched: 1 row(s)
hive> drop database testjira;
OK
Time taken: 0.194 seconds
hive>
host1:/opt/tools/hadoop3.1/hadoop-3.1.1/bin # ./hadoop fs -ls /user
18/09/27 18:07:32 WARN util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
host1:/opt/tools/hadoop3.1/hadoop-3.1.1/bin #
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96601 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96601/testReport)** for PR 22466 at commit [`597f8e6`](https://github.com/apache/spark/commit/597f8e6130965ffbfbdd94bfb4ce3e9c90eed794).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97158 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97158/testReport)** for PR 22466 at commit [`b837d48`](https://github.com/apache/spark/commit/b837d48f688a4fa5aa8766aa076855eb3a6bb97d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97614 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97614/testReport)** for PR 22466 at commit [`b091996`](https://github.com/apache/spark/commit/b09199614c8734c97baee077406d76fde3cb6f02).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97118 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97118/testReport)** for PR 22466 at commit [`c50e99b`](https://github.com/apache/spark/commit/c50e99b03bcda23762817aeb763971082da8989e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226655438
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,14 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (!externalCatalog.databaseExists(dbName) && fs.exists(dbPath)
+ && fs.listStatus(dbPath).nonEmpty) {
--- End diff --
if the behavior needs to be consistent over all the external catalog implementations(it should be), then I think putting it here is reasonable.
But listing files might be too expensive(e.g. several hourse), is there a better way to check if a directory is empty?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220795446
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
Please try the following cases:
- the directory does not exist and the parent directory does not exist too.
- the directory exists but empty
- the directory exists and non-empty
- the directory is not accessible
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22466
See JIRA, I don't think this should be merged.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96601/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r222855301
--- Diff: python/pyspark/sql/tests.py ---
@@ -351,7 +351,7 @@ def tearDown(self):
super(SQLTests, self).tearDown()
# tear down test_bucketed_write state
- self.spark.sql("DROP TABLE IF EXISTS pyspark_bucket")
+ self.spark.sql("DROP DATABASE IF EXISTS some_db CASCADE")
--- End diff --
why do we need to drop the database now?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]When database is dropped all the data ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r222891525
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -407,6 +407,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
test("create a managed table with the existing non-empty directory") {
withTable("tab1") {
+ Utils.createDirectory(spark.sessionState.conf.warehousePath)
--- End diff --
As per line 414 it is trying to create empty directory under warehouse path,this statement will fail as parent folder(ware house) does not exists
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r224667371
--- Diff: python/pyspark/sql/tests.py ---
@@ -2993,6 +2990,7 @@ def test_current_database(self):
AnalysisException,
"does_not_exist",
lambda: spark.catalog.setCurrentDatabase("does_not_exist"))
+ spark.sql("DROP DATABASE some_db")
--- End diff --
We should surround with try-finally?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220795176
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
Just want to confirm it what is the behavior of Hive (3.x) when we try to create a database in a non-empty file path?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22466
We should look at Spark documentation, and Hive, if any, to figure out what the right behavior is here. Spark generally follows Hive. See https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-CreateTableCreate/Drop/TruncateTable I think this is, further, conflating what `LOCATION` and `EXTERNAL` does. I agree that external DB files shouldn't be deleted, but not simply those specified by `LOCATION`. At least that is my understanding.
@yhuai or @cloud-fan or @clockfly might know more.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220553606
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
not sure if we should do it here or in the external catalog, cc @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96619 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96619/testReport)** for PR 22466 at commit [`597f8e6`](https://github.com/apache/spark/commit/597f8e6130965ffbfbdd94bfb4ce3e9c90eed794).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97158 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97158/testReport)** for PR 22466 at commit [`b837d48`](https://github.com/apache/spark/commit/b837d48f688a4fa5aa8766aa076855eb3a6bb97d).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
cc @gatorsmile if everything is okay ,this PR can be merged ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220564446
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
The reason I did here is, external catalog always have path (default or user specified),so we end up in checking the path always.This may be the costly operation if the file system is S3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226624218
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,14 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (!externalCatalog.databaseExists(dbName) && fs.exists(dbPath)
+ && fs.listStatus(dbPath).nonEmpty) {
--- End diff --
Oh, btw, I noticed that @gatorsmile is asking for doing the file existence check in the external catalog, not in the session catalog. Could you move this there?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
@cloud-fan @gatorsmile if everything is okay,can you please merge this PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536456
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -840,12 +840,19 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
}
test("create table in default db") {
- val catalog = spark.sessionState.catalog
- val tableIdent1 = TableIdentifier("tab1", None)
- createTable(catalog, tableIdent1)
- val expectedTableIdent = tableIdent1.copy(database = Some("default"))
- val expectedTable = generateTable(catalog, expectedTableIdent)
- checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ var tablePath: Option[URI] = None
+ try {
+ val catalog = spark.sessionState.catalog
+ val tableIdent1 = TableIdentifier("tab1", None)
+ createTable(catalog, tableIdent1)
+ val expectedTableIdent = tableIdent1.copy(database = Some("default"))
+ val expectedTable = generateTable(catalog, expectedTableIdent)
+ tablePath = Some(expectedTable.location)
+ checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ } finally {
+ // This is external table,so it is required to deleted the path
--- End diff --
`it is required to delete`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96736 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96736/testReport)** for PR 22466 at commit [`f05d7c1`](https://github.com/apache/spark/commit/f05d7c14f3942743485c8b78ac40ee7e1d3bafa9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r219442479
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2348,4 +2348,41 @@ class HiveDDLSuite
}
}
}
+
+ test("SPARK-25464 Drop Database check location after dropping external Database") {
+ val catalog = spark.sessionState.catalog
+ val dbName = "dbwithcustomlocation"
+ withTempDir { tmpDir =>
+ val path = new Path(tmpDir.getCanonicalPath)
+ val fs = path.getFileSystem(new Configuration)
+ try {
+ sql(s"CREATE DATABASE $dbName Location '${ path.toUri }'")
+ val db1 = catalog.getDatabaseMetadata(dbName)
+ assert(db1.properties.getOrElse("isExternal","empty").equals("true"))
+ val client =
+ spark.sharedState.externalCatalog.unwrapped.asInstanceOf[HiveExternalCatalog].client
+ client.dropDatabase(dbName,false,true)
+ assert(fs.exists(path))
+ assert(!catalog.databaseExists(dbName))
+ }
+ finally {
--- End diff --
please pull finally to 1 level up inline with the close brace of try
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97615 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97615/testReport)** for PR 22466 at commit [`570c5b7`](https://github.com/apache/spark/commit/570c5b7dece38814be5ece45086289e9594208e6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96929 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96929/testReport)** for PR 22466 at commit [`7577a5a`](https://github.com/apache/spark/commit/7577a5aecd92368cc5ac1b7035d3a1a571dea3fa).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96689/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97550 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97550/testReport)** for PR 22466 at commit [`d862591`](https://github.com/apache/spark/commit/d862591c20ef1d1536d069dcc4f3220ae232c702).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
can one of the admin ask for retest please ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22466
I'm not sure if there is a concept called "external database" in Hive...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
cc @cloud-fan @srowen I have updated the code,Please review
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96750 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96750/testReport)** for PR 22466 at commit [`63dc53a`](https://github.com/apache/spark/commit/63dc53aba545fecfe5301c7ea0b8305e60e20496).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r220795973
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -66,6 +66,19 @@ case class CreateDatabaseCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
+ // SPARK-25464 fail if DB location exists and is not empty
+ if (path.isDefined) {
--- End diff --
I think we should only block the last two cases. I also prefer to doing it in the external catalog. In the future, if we support catalog federation, different external catalog can define their own behavior.
Listing files in S3 is expensive, but create a new database is not frequent. I think our users can accept this cost.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97548 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97548/testReport)** for PR 22466 at commit [`675ab8c`](https://github.com/apache/spark/commit/675ab8c60f58f0326b5a98d7205028220d64d4f4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97615 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97615/testReport)** for PR 22466 at commit [`570c5b7`](https://github.com/apache/spark/commit/570c5b7dece38814be5ece45086289e9594208e6).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #97182 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97182/testReport)** for PR 22466 at commit [`5d95a09`](https://github.com/apache/spark/commit/5d95a09122436217a8a6e891cfb96babfe49fb2b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22466
**[Test build #96768 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96768/testReport)** for PR 22466 at commit [`3c2831b`](https://github.com/apache/spark/commit/3c2831bf70d4ed83414625fd3e4b7607624d09fc).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]When database is dropped all the data ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r223570144
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,16 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (fs.exists(dbPath)) {
+ val fileStatus = fs.listStatus(dbPath)
+ if (fileStatus.nonEmpty) {
--- End diff --
if (fs.exists(dbPath) && fs.listStatus(dbPath).nonEmpty) {
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r221307039
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -207,6 +207,16 @@ class SessionCatalog(
"you cannot create a database with this name.")
}
validateName(dbName)
+ // SPARK-25464 fail if DB location exists and is not empty
+ val dbPath = new Path(dbDefinition.locationUri)
+ val fs = dbPath.getFileSystem(hadoopConf)
+ if (fs.exists(dbPath)) {
+ val fileStatus = fs.listStatus(dbPath)
+ if (0 != fileStatus.length) {
--- End diff --
`if (fileStatus.nonEmpty)`? at least, flip the comparison. You can add the location to the exception message, maybe.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r219443041
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -321,8 +321,19 @@ private[hive] class HiveClientImpl(
override def dropDatabase(
name: String,
ignoreIfNotExists: Boolean,
- cascade: Boolean): Unit = withHiveState {
- client.dropDatabase(name, true, ignoreIfNotExists, cascade)
+ cascade: Boolean): Unit = {
+ var isExternalDB = false
+ try {
+ val db: CatalogDatabase = getDatabase(name)
+ isExternalDB = db.properties.getOrElse("isExternal", "false").matches("true")
+ }
+ catch {
--- End diff --
please pull catch to 1 level up inline with the close brace of try
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97118/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536304
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -840,12 +840,19 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
}
test("create table in default db") {
- val catalog = spark.sessionState.catalog
- val tableIdent1 = TableIdentifier("tab1", None)
- createTable(catalog, tableIdent1)
- val expectedTableIdent = tableIdent1.copy(database = Some("default"))
- val expectedTable = generateTable(catalog, expectedTableIdent)
- checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ var tablePath: Option[URI] = None
+ try {
+ val catalog = spark.sessionState.catalog
+ val tableIdent1 = TableIdentifier("tab1", None)
+ createTable(catalog, tableIdent1)
+ val expectedTableIdent = tableIdent1.copy(database = Some("default"))
+ val expectedTable = generateTable(catalog, expectedTableIdent)
+ tablePath = Some(expectedTable.location)
+ checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ } finally {
+ // This is external table,so it is required to deleted the path
--- End diff --
tiny nit: `e,` -> `e ,`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/22466
@cloud-fan @gatorsmile all the testcases are passed and review comments are addressed,can you help me to merge this PR please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22466
Regarding the potentially high cost of file listing, `CREATE DATABASE` is not a frequent operation. The cost is high only if the target directory is non-empty with many many files. We are blocking users from creating such a database. Thus, the cost is not a big deal I think.
We need to list this behavior change in the SQL migration guide.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL]On dropping the Database it will drop ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96929/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r222855237
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -407,6 +407,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
test("create a managed table with the existing non-empty directory") {
withTable("tab1") {
+ Utils.createDirectory(spark.sessionState.conf.warehousePath)
--- End diff --
Why do we need to create the directory first?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL] Create Database to the locatio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r226536442
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -840,12 +840,19 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
}
test("create table in default db") {
- val catalog = spark.sessionState.catalog
- val tableIdent1 = TableIdentifier("tab1", None)
- createTable(catalog, tableIdent1)
- val expectedTableIdent = tableIdent1.copy(database = Some("default"))
- val expectedTable = generateTable(catalog, expectedTableIdent)
- checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ var tablePath: Option[URI] = None
+ try {
+ val catalog = spark.sessionState.catalog
+ val tableIdent1 = TableIdentifier("tab1", None)
+ createTable(catalog, tableIdent1)
+ val expectedTableIdent = tableIdent1.copy(database = Some("default"))
+ val expectedTable = generateTable(catalog, expectedTableIdent)
+ tablePath = Some(expectedTable.location)
+ checkCatalogTables(expectedTable, catalog.getTableMetadata(tableIdent1))
+ } finally {
+ // This is external table,so it is required to deleted the path
--- End diff --
`this is an external table`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22466: [SPARK-25464][SQL]On dropping the Database it wil...
Posted by sujith71955 <gi...@git.apache.org>.
Github user sujith71955 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22466#discussion_r219440456
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -67,12 +67,15 @@ case class CreateDatabaseCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
+ // if the path is specified by the user then need to consider DB as external DB
+ // User will take care of deleting the path after DB is dropped
--- End diff --
nit: user shall take care of deleting the data from external location.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22466
Btw, what if `create database if not exists ...`? Seems like an exception will be thrown if the table exists even if we specify `if not exists`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22466: [SPARK-25464][SQL] Create Database to the location,only ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22466
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97550/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org