You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/05/01 05:45:08 UTC
[GitHub] [spark] vinodkc opened a new pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
vinodkc opened a new pull request #32411:
URL: https://github.com/apache/spark/pull/32411
### What changes were proposed in this pull request?
CTAS with location clause acts as an insert overwrite. This can cause problems when there subdirectories within a location directory.
This causes some users to accidentally wipe out directories with very important data. We should not allow CTAS with location to a non-empty directory.
### Why are the changes needed?
Hive already handled this scenario: HIVE-11319
Steps to reproduce:
sql("""create external table `demo_CTAS`( `comment` string) PARTITIONED BY (`col1` string, `col2` string) STORED AS parquet location '/tmp/u1/demo_CTAS'""")
sql("""INSERT OVERWRITE TABLE demo_CTAS partition (col1='1',col2='1') VALUES ('abc')""")
sql("select* from demo_CTAS").show
sql("""create table ctas1 location '/tmp/u2/ctas1' as select * from demo_CTAS""")
sql("select* from ctas1").show
sql("""create table ctas2 location '/tmp/u2' as select * from demo_CTAS""")
Before the fix: Both create table operations will succeed. But values in table ctas1 will be replaced by ctas2 accidentally.
After the fix: `create table ctas2...` will throw AnalysisException
org.apache.spark.sql.AnalysisException: CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory /tmp/u2 .
### Does this PR introduce _any_ user-facing change?
Yes, if the location directory is not empty, CTAS with location will throw AnalysisException
sql("""create table ctas2 location '/tmp/u2' as select * from demo_CTAS""")
org.apache.spark.sql.AnalysisException: CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory /tmp/u2 .
### How was this patch tested?
Test case added in SQLQuerySuite.scala
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832880289
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r635099518
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,19 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite && !SQLConf.get.allowNonEmptyLocationInCTAS) {
Review comment:
For SQL CTAS, the mode is always `ErrorIfExists`.
In that case, I'll change the condition as `if (saveMode == SaveMode.**ErrorIfExists** && !SQLConf.get.allowNonEmptyLocationInCTAS)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830565568
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42645/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839169084
**[Test build #138385 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138385/testReport)** for PR 32411 at commit [`a37aba6`](https://github.com/apache/spark/commit/a37aba6c7501f6425d7651be29b4a7c252722b40).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830563487
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830565568
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42645/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830619228
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42649/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830626858
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138128/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r635099518
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,19 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite && !SQLConf.get.allowNonEmptyLocationInCTAS) {
Review comment:
For SQL CTAS, the mode is always `ErrorIfExists`, and spark just checks whether the table exists or not. It was not checking non-empty data path.
In that case, I'll change the condition as `if (saveMode == SaveMode.**ErrorIfExists** && !SQLConf.get.allowNonEmptyLocationInCTAS)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832891227
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42700/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630278821
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
Review comment:
Yes, Hive does the same thing. [HIVE-11319]
ref: https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java#L13615
Later Another JIRA HIVE-15367 added one more level of check for target location, which only contains the staging-dirs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841040367
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43069/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844740216
Thank you, @vinodkc and all!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841017050
**[Test build #138550 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138550/testReport)** for PR 32411 at commit [`67d5a78`](https://github.com/apache/spark/commit/67d5a78b0c8b51484ce607896797d0d6129b7ec6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840616215
**[Test build #138508 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138508/testReport)** for PR 32411 at commit [`c3f909e`](https://github.com/apache/spark/commit/c3f909ecaf015d39a6fc759e135ed7c93286b630).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832855291
**[Test build #138179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138179/testReport)** for PR 32411 at commit [`8f02c0f`](https://github.com/apache/spark/commit/8f02c0f1f2fd907a68239b372538dd6c100360b4).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830659943
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42652/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r633635249
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,19 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite && !SQLConf.get.allowNonEmptyLocationInCTAS) {
Review comment:
Can we add a bit more comment to explain the save mode here? For SQL CTAS, the mode is always append?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -235,10 +235,10 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
}
test("create table using as select - with overridden max number of buckets") {
- def createTableSql(numBuckets: Int): String =
+ def createTableSql(tablePath: String, numBuckets: Int): String =
s"""
|CREATE TABLE t USING PARQUET
- |OPTIONS (PATH '${path.toURI}')
+ |OPTIONS (PATH '$tablePath')
Review comment:
is this change necessary? it `beforeEach` we create a new temp dir and assign it to `path`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632024938
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(
+ SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS.key -> allowNonEmptyDirFlag) {
+ withTempDir { dir =>
+ val tempLocation = dir.toURI.toString
+ withTable("ctas1", "ctas_with_existing_location") {
+ if (!spark.conf.get(SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS)) {
+ intercept[AnalysisException] {
Review comment:
Please check the exact error message. Otherwise, we may miss some future regressions due to that another `AnalysisException`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838776197
**[Test build #138385 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138385/testReport)** for PR 32411 at commit [`a37aba6`](https://github.com/apache/spark/commit/a37aba6c7501f6425d7651be29b4a7c252722b40).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840559936
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43031/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844292368
**[Test build #138709 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138709/testReport)** for PR 32411 at commit [`bafe546`](https://github.com/apache/spark/commit/bafe546ae09a9a0c0e118aef3d4e7f05b30c8ed3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r635076302
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -235,10 +235,10 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
}
test("create table using as select - with overridden max number of buckets") {
- def createTableSql(numBuckets: Int): String =
+ def createTableSql(tablePath: String, numBuckets: Int): String =
s"""
|CREATE TABLE t USING PARQUET
- |OPTIONS (PATH '${path.toURI}')
+ |OPTIONS (PATH '$tablePath')
Review comment:
Yes, in this test case, the function `createTableSql ` will be called twice inside `Seq(100001, maxNrBuckets).foreach(...)`. As this PR doesn't allow CTAS with a non-empty location, the second call of `createTableSql` will fail if the same `path` used. That is why for each iteration a new table location is created
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838844413
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840637497
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138508/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc edited a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc edited a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830734863
A gentle ping @wangyum @gatorsmile @dongjoon-hyun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631705217
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830617280
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830611204
**[Test build #138128 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138128/testReport)** for PR 32411 at commit [`293cab3`](https://github.com/apache/spark/commit/293cab33152994f119e80d6e7496e1139896dd72).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839180400
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138385/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32411:
URL: https://github.com/apache/spark/pull/32411
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840376552
The new behavior makes sense to me. I'm wondering what shall we do for data source tables. e.g.
```
CREATE TABLE t USING jdbc OPTIONS (... dbtable="foo") AS SELECT ...
```
The "foo" in JDBC is not empty, what shall we do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840559936
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43031/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631813498
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
Review comment:
You can `Seq(false, true)` .. and do `allowNonEmptyDirFlag.toString` at `withSQLConf`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844027984
**[Test build #138709 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138709/testReport)** for PR 32411 at commit [`bafe546`](https://github.com/apache/spark/commit/bafe546ae09a9a0c0e118aef3d4e7f05b30c8ed3).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832430678
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138156/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830573441
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138124/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r626729769
##########
File path: docs/sql-migration-guide.md
##########
@@ -78,7 +78,9 @@ license: |
- In Spark 3.2, the timestamps subtraction expression such as `timestamp '2021-03-31 23:48:00' - timestamp '2021-01-01 00:00:00'` returns values of `DayTimeIntervalType`. In Spark 3.1 and earlier, the type of the same expression is `CalendarIntervalType`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.interval.enabled` to `true`.
- In Spark 3.2, `CREATE TABLE .. LIKE ..` command can not use reserved properties. You need their specific clauses to specify them, for example, `CREATE TABLE test1 LIKE test LOCATION 'some path'`. You can set `spark.sql.legacy.notReserveProperties` to `true` to ignore the `ParseException`, in this case, these properties will be silently removed, for example: `TBLPROPERTIES('owner'='yao')` will have no effect. In Spark version 3.1 and below, the reserved properties can be used in `CREATE TABLE .. LIKE ..` command but have no side effects, for example, `TBLPROPERTIES('location'='/tmp')` does not change the location of the table but only create a headless property just like `'a'='b'`.
-
+
+ - In Spark 3.2, `CREATE TABLE AS SELECT` with non-empty `LOCATION` will throw `AnalysisException`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.ctas.allowNonEmptyLocation` to `true`.
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830619228
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42649/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832855291
**[Test build #138179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138179/testReport)** for PR 32411 at commit [`8f02c0f`](https://github.com/apache/spark/commit/8f02c0f1f2fd907a68239b372538dd6c100360b4).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630301266
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
+ .internal()
+ .doc("When false, Create Table As Location Select will throw Analysis Exception, " +
+ "if the location is non empty.")
+ .version("3.2")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630102713
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
Review comment:
The problem here is that it does one more extra listing which can make the query slow (e.g., S3). Does Hive do the same thing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844743155
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830652565
**[Test build #138131 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138131/testReport)** for PR 32411 at commit [`62e5aab`](https://github.com/apache/spark/commit/62e5aab6bc8ccffdf7c08df54bb4ed6a6bd743b1).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-833017822
**[Test build #138179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138179/testReport)** for PR 32411 at commit [`8f02c0f`](https://github.com/apache/spark/commit/8f02c0f1f2fd907a68239b372538dd6c100360b4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840468792
**[Test build #138508 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138508/testReport)** for PR 32411 at commit [`c3f909e`](https://github.com/apache/spark/commit/c3f909ecaf015d39a6fc759e135ed7c93286b630).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838342459
A gentle ping @wangyum @gatorsmile @dongjoon-hyun @HyukjinKwon @AngersZhuuuu
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840710706
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138511/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830626858
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138128/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840508562
> The new behavior makes sense to me. I'm wondering what shall we do for data source tables. e.g.
>
> ```
> CREATE TABLE t USING jdbc OPTIONS (... dbtable="foo") AS SELECT ...
> ```
>
> The "foo" in JDBC is not empty, what shall we do?
Yes, Currently nonempty table "foo" will be overwritten. Should we handle that scenario also in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630742282
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
+ .internal()
+ .doc("When false, Create Table As Location Select will throw Analysis Exception, " +
+ "if the location is non empty.")
Review comment:
```suggestion
.doc("When false, CTAS with LOCATION throws an analysis exception if the "
"location is not empty." +
```
##########
File path: docs/sql-migration-guide.md
##########
@@ -78,7 +78,9 @@ license: |
- In Spark 3.2, the timestamps subtraction expression such as `timestamp '2021-03-31 23:48:00' - timestamp '2021-01-01 00:00:00'` returns values of `DayTimeIntervalType`. In Spark 3.1 and earlier, the type of the same expression is `CalendarIntervalType`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.interval.enabled` to `true`.
- In Spark 3.2, `CREATE TABLE .. LIKE ..` command can not use reserved properties. You need their specific clauses to specify them, for example, `CREATE TABLE test1 LIKE test LOCATION 'some path'`. You can set `spark.sql.legacy.notReserveProperties` to `true` to ignore the `ParseException`, in this case, these properties will be silently removed, for example: `TBLPROPERTIES('owner'='yao')` will have no effect. In Spark version 3.1 and below, the reserved properties can be used in `CREATE TABLE .. LIKE ..` command but have no side effects, for example, `TBLPROPERTIES('location'='/tmp')` does not change the location of the table but only create a headless property just like `'a'='b'`.
-
+
+ - In Spark 3.2, `CREATE TABLE AS SELECT` with non-empty `LOCATION` will throw `AnalysisException`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.ctas.allowNonEmptyLocation` to `true`.
+
Review comment:
I think you could just add this in the last.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -249,16 +249,18 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
// Within the new limit
Seq(100001, maxNrBuckets).foreach(numBuckets => {
- withTable("t") {
- sql(createTableSql(numBuckets))
- val table = catalog.getTableMetadata(TableIdentifier("t"))
- assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
- }
+ withTempDir(tempDir => {
+ withTable("t") {
+ sql(createTableSql(tempDir.toURI, numBuckets))
+ val table = catalog.getTableMetadata(TableIdentifier("t"))
+ assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
+ }
+ })
Review comment:
```suggestion
}
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
Review comment:
```suggestion
s"'$tempLocation/ctas1'")
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_CTAS_NON_EMPTY_LOCATION.key -> allowNonEmptyDirFlag) {
Review comment:
```suggestion
withSQLConf(
SQLConf.CONVERT_CTAS.key -> convertCTASFlag.toString,
SQLConf.ALLOW_CTAS_NON_EMPTY_LOCATION.key -> allowNonEmptyDirFlag.toString) {
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
Review comment:
Can we use Scala's multiline style with vertical bars?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
Review comment:
I think you can make it as a single condition:
```scala
if (fs.exists(filePath) &&
fs.getFileStatus(filePath).isDirectory &&
fs.listStatus(filePath).length != 0) {
...
}
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table" +
Review comment:
I would say something like this:
> The directory ${tablePath} was not empty but CTAS with LOCATION requires the location to be empty.
See also https://spark.apache.org/error-message-guidelines.html
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -249,16 +249,18 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
// Within the new limit
Seq(100001, maxNrBuckets).foreach(numBuckets => {
- withTable("t") {
- sql(createTableSql(numBuckets))
- val table = catalog.getTableMetadata(TableIdentifier("t"))
- assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
- }
+ withTempDir(tempDir => {
Review comment:
```suggestion
withTempDir { tempDir =>
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
Review comment:
can the file status be `null`?
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
Review comment:
```suggestion
Seq(false, true).foreach { convertCTASFlag =>
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -249,16 +249,18 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
// Within the new limit
Seq(100001, maxNrBuckets).foreach(numBuckets => {
- withTable("t") {
- sql(createTableSql(numBuckets))
- val table = catalog.getTableMetadata(TableIdentifier("t"))
- assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
- }
+ withTempDir(tempDir => {
+ withTable("t") {
+ sql(createTableSql(tempDir.toURI, numBuckets))
Review comment:
```suggestion
sql(createTableSql(tempDir.toURI.toString, numBuckets))
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_CTAS_NON_EMPTY_LOCATION.key -> allowNonEmptyDirFlag) {
+ withTempDir { dir =>
+ val tempLocation = dir.toURI.getPath.stripSuffix("/")
Review comment:
```suggestion
val tempLocation = dir.toURI.toString
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -249,16 +249,18 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
// Within the new limit
Seq(100001, maxNrBuckets).foreach(numBuckets => {
- withTable("t") {
- sql(createTableSql(numBuckets))
- val table = catalog.getTableMetadata(TableIdentifier("t"))
- assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
- }
+ withTempDir(tempDir => {
+ withTable("t") {
+ sql(createTableSql(tempDir.toURI, numBuckets))
+ val table = catalog.getTableMetadata(TableIdentifier("t"))
+ assert(table.bucketSpec == Option(BucketSpec(numBuckets, Seq("a"), Seq("b"))))
+ }
+ })
})
// Over the new limit
withTable("t") {
- val e = intercept[AnalysisException](sql(createTableSql(maxNrBuckets + 1)))
+ val e = intercept[AnalysisException](sql(createTableSql(path.toURI, maxNrBuckets + 1)))
Review comment:
```suggestion
val e = intercept[AnalysisException](sql(createTableSql(path.toURI.toString, maxNrBuckets + 1)))
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
Review comment:
```suggestion
buildConf("spark.sql.legacy.allowNonEmptyLocationInCTAS")
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
Review comment:
```suggestion
Seq(false, true).foreach { allowNonEmptyDirFlag =>
```
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
Review comment:
```suggestion
s"LOCATION '$tempLocation' " +
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -235,10 +235,10 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
}
test("create table using as select - with overridden max number of buckets") {
- def createTableSql(numBuckets: Int): String =
+ def createTableSql(tablePath: java.net.URI, numBuckets: Int): String =
Review comment:
```suggestion
def createTableSql(tablePath: String, numBuckets: Int): String =
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
##########
@@ -166,6 +166,9 @@ case class CreateDataSourceTableAsSelectCommand(
saveDataIntoTable(
sparkSession, table, table.storage.locationUri, child, SaveMode.Append, tableExists = true)
} else {
+ table.storage.locationUri.foreach { p =>
+ DataWritingCommand.assertEmptyRootPath(p, mode, sparkSession)
Review comment:
```suggestion
DataWritingCommand.assertEmptyRootPath(p, mode)
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
Review comment:
```suggestion
!SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
Review comment:
```suggestion
def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode) {
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
Review comment:
can `fs` be `null`?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
Review comment:
Can we pass Hadoop configuration as an argument for `assertEmptyRootPath` function?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
Review comment:
```suggestion
if (fs != null && fs.exists(filePath)) {
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
Review comment:
```suggestion
if (locStats != null && locStats.isDirectory) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r633635249
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,19 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite && !SQLConf.get.allowNonEmptyLocationInCTAS) {
Review comment:
Can we add a bit more comment to explain the save mode here? For SQL CTAS, the mode is always append?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844068932
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43230/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830691948
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138131/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839482908
Okay, the fix makes sense to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630749962
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table" +
Review comment:
We should also probably mention the legacy configuration here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838776197
**[Test build #138385 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138385/testReport)** for PR 32411 at commit [`a37aba6`](https://github.com/apache/spark/commit/a37aba6c7501f6425d7651be29b4a7c252722b40).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840489583
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43028/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838856363
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42908/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844736709
thanks, merging to master!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830659943
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42652/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-833057475
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138179/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832379971
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42677/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832327918
**[Test build #138156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138156/testReport)** for PR 32411 at commit [`1a9a86f`](https://github.com/apache/spark/commit/1a9a86fc84dc8c209f6fd7cc398b2814d94d4dd2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631706750
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
+ .internal()
+ .doc("When false, Create Table As Location Select will throw Analysis Exception, " +
+ "if the location is non empty.")
Review comment:
Done
##########
File path: docs/sql-migration-guide.md
##########
@@ -78,7 +78,9 @@ license: |
- In Spark 3.2, the timestamps subtraction expression such as `timestamp '2021-03-31 23:48:00' - timestamp '2021-01-01 00:00:00'` returns values of `DayTimeIntervalType`. In Spark 3.1 and earlier, the type of the same expression is `CalendarIntervalType`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.interval.enabled` to `true`.
- In Spark 3.2, `CREATE TABLE .. LIKE ..` command can not use reserved properties. You need their specific clauses to specify them, for example, `CREATE TABLE test1 LIKE test LOCATION 'some path'`. You can set `spark.sql.legacy.notReserveProperties` to `true` to ignore the `ParseException`, in this case, these properties will be silently removed, for example: `TBLPROPERTIES('owner'='yao')` will have no effect. In Spark version 3.1 and below, the reserved properties can be used in `CREATE TABLE .. LIKE ..` command but have no side effects, for example, `TBLPROPERTIES('location'='/tmp')` does not change the location of the table but only create a headless property just like `'a'='b'`.
-
+
+ - In Spark 3.2, `CREATE TABLE AS SELECT` with non-empty `LOCATION` will throw `AnalysisException`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.ctas.allowNonEmptyLocation` to `true`.
+
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844027984
**[Test build #138709 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138709/testReport)** for PR 32411 at commit [`bafe546`](https://github.com/apache/spark/commit/bafe546ae09a9a0c0e118aef3d4e7f05b30c8ed3).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632289902
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
+ fs.listStatus(filePath).length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory " +
+ s"${tablePath} . To disable the behavior, " +
+ s"set 'spark.sql.legacy.allowNonEmptyLocationInCTAS to true.")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631758038
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
Review comment:
withSQLConf accepts pairs of String (String, String), passing (String, Boolean) will fail to compile
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830652565
**[Test build #138131 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138131/testReport)** for PR 32411 at commit [`62e5aab`](https://github.com/apache/spark/commit/62e5aab6bc8ccffdf7c08df54bb4ed6a6bd743b1).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631783283
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
+ fs.listStatus(filePath).length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory " +
+ s"${tablePath} . To disable the behavior, " +
+ s"set 'spark.sql.legacy.allowNonEmptyLocationInCTAS to true.")
Review comment:
nit: use `SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS.key` instead of hardcode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r624870695
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(locationUri: Option[URI], configuration: Configuration) {
+ locationUri.fold() { tablePathUri: URI =>
+ val tablePath = tablePathUri.toString
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(configuration)
+ if(fs.exists(filePath)) {
+ val cs = fs.getContentSummary(filePath)
Review comment:
This kind of listing can potentially be expensive, and looks `getContentSummary` is recursive too. I think we should at least guard by a legacy configuration with updating the migration guide at https://github.com/apache/spark/blob/master/docs/sql-migration-guide.md
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840526343
> Should we handle that scenario also in this PR?
Let's leave it for now. I don't know how to check "empty" with data source v1 and may need more thinking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630102272
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
+ .internal()
+ .doc("When false, Create Table As Location Select will throw Analysis Exception, " +
+ "if the location is non empty.")
+ .version("3.2")
Review comment:
3.2.0 to be consistent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r624869835
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
##########
@@ -166,6 +166,10 @@ case class CreateDataSourceTableAsSelectCommand(
saveDataIntoTable(
sparkSession, table, table.storage.locationUri, child, SaveMode.Append, tableExists = true)
} else {
+ if (mode != SaveMode.Overwrite) {
+ DataWritingCommand.assertEmptyRootPath(table.storage.locationUri,
Review comment:
I would do:
```scala
table.storage.locationUri.foreach(
p => assertEmptyRootPath(p, sparkSession.sparkContext.hadoopConfiguration))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844294152
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138709/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840522857
**[Test build #138511 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138511/testReport)** for PR 32411 at commit [`0d81aa4`](https://github.com/apache/spark/commit/0d81aa4dde9108233853cab804998aaaab48f998).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r626731935
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,24 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyDirPath) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table" +
+ s" with location to a non-empty directory " +
+ s"${tablePath} .")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830573441
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138124/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840495349
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43028/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830553307
**[Test build #138124 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138124/testReport)** for PR 32411 at commit [`7e996d6`](https://github.com/apache/spark/commit/7e996d6b6e92c5de4697d567e47a2e987551829c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839483583
The fix makes sense to me. cc @cloud-fan too FYI. It is a behaviour change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840493434
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43028/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832379971
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42677/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632288384
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
Review comment:
Done
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
Review comment:
Done
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
+ fs.listStatus(filePath).length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory " +
+ s"${tablePath} . To disable the behavior, " +
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632288269
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(
+ SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS.key -> allowNonEmptyDirFlag) {
+ withTempDir { dir =>
+ val tempLocation = dir.toURI.toString
+ withTable("ctas1", "ctas_with_existing_location") {
+ if (!spark.conf.get(SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS)) {
+ intercept[AnalysisException] {
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839482908
Okay, the fix makes sense to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832416948
**[Test build #138156 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138156/testReport)** for PR 32411 at commit [`1a9a86f`](https://github.com/apache/spark/commit/1a9a86fc84dc8c209f6fd7cc398b2814d94d4dd2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832379819
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844179653
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43230/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631812857
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
Review comment:
You can do `allowNonEmptyDirFlag.toString`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632023031
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
Review comment:
indentation?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
Review comment:
indentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830978432
cc @AngersZhuuuu
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830734863
A gentle ping @wangyum @@gatorsmile @dongjoon-hyun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631783736
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(
+ SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS.key -> allowNonEmptyDirFlag) {
+ withTempDir { dir =>
+ val tempLocation = dir.toURI.toString
+ withTable("ctas1", "ctas_with_existing_location") {
+ if (!spark.conf.get(SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS)) {
Review comment:
nit: `if (allowNonEmptyDirFlag == "false")`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631622569
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table" +
Review comment:
In Hive , https://github.com/apache/hive/blob/master/common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java#L408
Error message is `CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory.`
To be consistent, shall we show this message?
"CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory. To disable the behavior, set 'spark.sql.legacy.allowNonEmptyLocationInCTAS to true"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841161198
**[Test build #138550 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138550/testReport)** for PR 32411 at commit [`67d5a78`](https://github.com/apache/spark/commit/67d5a78b0c8b51484ce607896797d0d6129b7ec6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631550114
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,38 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION " +
+ s"'file:$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"CREATE TABLE ctas_with_existing_location stored as rcfile " +
+ s"LOCATION 'file:$tempLocation' " +
+ s"AS SELECT key k, value FROM src ORDER BY k, value")
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
Review comment:
withSQLConf accepts pairs of String (String, String), passing (String, Boolean) will fail to compile
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631706428
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
Review comment:
removed null check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830573283
**[Test build #138124 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138124/testReport)** for PR 32411 at commit [`7e996d6`](https://github.com/apache/spark/commit/7e996d6b6e92c5de4697d567e47a2e987551829c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r630278821
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
Review comment:
Yes, Hive does the same thing. [HIVE-11319]
ref: https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java#L13615
The liststatus call will be slow when the location has data, which is a failure condition as per this PR. So in the normal case, location Dir will be empty and liststatus won't be slow.
Later Another JIRA HIVE-15367 added one more level of check for target location, which only contains the staging-dirs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632021958
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +99,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, hadoopConf: Configuration) {
+ if (saveMode != SaveMode.Overwrite &&
+ !SQLConf.get.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(hadoopConf)
+ if (fs.exists(filePath) &&
+ fs.getFileStatus(filePath).isDirectory &&
+ fs.listStatus(filePath).length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory " +
+ s"${tablePath} . To disable the behavior, " +
Review comment:
Shall we have a more explicit message because `To disable the behavior` doesn't imply the overwrite behavior? Maybe, the following is better?
```scala
- To disable the behavior,
- To allow overwriting the existing non-empty directory,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632289297
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
+ Seq("false", "true").foreach { allowNonEmptyDirFlag =>
+ withSQLConf(
+ SQLConf.CONVERT_CTAS.key -> convertCTASFlag,
+ SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS.key -> allowNonEmptyDirFlag) {
+ withTempDir { dir =>
+ val tempLocation = dir.toURI.toString
+ withTable("ctas1", "ctas_with_existing_location") {
+ if (!spark.conf.get(SQLConf.ALLOW_NON_EMPTY_LOCATION_IN_CTAS)) {
Review comment:
Done, changed type of val `allowNonEmptyDirFlag` to Boolean
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841040367
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43069/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-833057475
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138179/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844179653
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43230/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r625564743
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(locationUri: Option[URI], configuration: Configuration) {
+ locationUri.fold() { tablePathUri: URI =>
+ val tablePath = tablePathUri.toString
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(configuration)
+ if(fs.exists(filePath)) {
+ val cs = fs.getContentSummary(filePath)
Review comment:
As fs.getContentSummary() is a costly operation compared to fs.listStatus(), now I used fs.listStatus(...).listStatus !=0
Also added new legacy configuration settings and updated the SQL migration guide
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830691400
**[Test build #138131 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138131/testReport)** for PR 32411 at commit [`62e5aab`](https://github.com/apache/spark/commit/62e5aab6bc8ccffdf7c08df54bb4ed6a6bd743b1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832403467
A gentle ping @HyukjinKwon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832471024
Thanks for your work, this issue we hit it for several times, always user write wrong location path and directly delete other table or database's data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841038512
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43069/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840552144
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43031/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844294152
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138709/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841017050
**[Test build #138550 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138550/testReport)** for PR 32411 at commit [`67d5a78`](https://github.com/apache/spark/commit/67d5a78b0c8b51484ce607896797d0d6129b7ec6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-844146948
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43230/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832327918
**[Test build #138156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138156/testReport)** for PR 32411 at commit [`1a9a86f`](https://github.com/apache/spark/commit/1a9a86fc84dc8c209f6fd7cc398b2814d94d4dd2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830623839
**[Test build #138128 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138128/testReport)** for PR 32411 at commit [`293cab3`](https://github.com/apache/spark/commit/293cab33152994f119e80d6e7496e1139896dd72).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-839180400
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138385/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r626314340
##########
File path: docs/sql-migration-guide.md
##########
@@ -78,7 +78,9 @@ license: |
- In Spark 3.2, the timestamps subtraction expression such as `timestamp '2021-03-31 23:48:00' - timestamp '2021-01-01 00:00:00'` returns values of `DayTimeIntervalType`. In Spark 3.1 and earlier, the type of the same expression is `CalendarIntervalType`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.interval.enabled` to `true`.
- In Spark 3.2, `CREATE TABLE .. LIKE ..` command can not use reserved properties. You need their specific clauses to specify them, for example, `CREATE TABLE test1 LIKE test LOCATION 'some path'`. You can set `spark.sql.legacy.notReserveProperties` to `true` to ignore the `ParseException`, in this case, these properties will be silently removed, for example: `TBLPROPERTIES('owner'='yao')` will have no effect. In Spark version 3.1 and below, the reserved properties can be used in `CREATE TABLE .. LIKE ..` command but have no side effects, for example, `TBLPROPERTIES('location'='/tmp')` does not change the location of the table but only create a headless property just like `'a'='b'`.
-
+
+ - In Spark 3.2, `CREATE TABLE AS SELECT` with non-empty `LOCATION` will throw `AnalysisException`. To restore the behavior before Spark 3.2, you can set `spark.sql.legacy.ctas.allowNonEmptyLocation` to `true`.
Review comment:
> with non-empty `LOCATION`
with non-empty `LOCATION` should use one space.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -3707,6 +3716,8 @@ class SQLConf extends Serializable with Logging {
def allowStarWithSingleTableIdentifierInCount: Boolean =
getConf(SQLConf.ALLOW_STAR_WITH_SINGLE_TABLE_IDENTIFIER_IN_COUNT)
+ def allowCreateTableAsSelectNonEmptyDirPath: Boolean = getConf(SQLConf.ALLOW_CTAS_NON_EMPTY_PATH)
Review comment:
allowCreateTableAsSelectNonEmptyLocation?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_PATH =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
Review comment:
ALLOW_CTAS_NON_EMPTY_LOCATION? should we keep consistence?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,24 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyDirPath) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
+ val lStats = fs.listStatus(filePath)
+ if(lStats != null && lStats.length != 0) {
+ throw new AnalysisException(
+ s"CREATE-TABLE-AS-SELECT cannot create table" +
+ s" with location to a non-empty directory " +
+ s"${tablePath} .")
Review comment:
Only two line should be ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841162182
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138550/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-838856363
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42908/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840710706
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138511/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840697097
**[Test build #138511 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138511/testReport)** for PR 32411 at commit [`0d81aa4`](https://github.com/apache/spark/commit/0d81aa4dde9108233853cab804998aaaab48f998).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-841162182
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138550/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840522857
**[Test build #138511 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138511/testReport)** for PR 32411 at commit [`0d81aa4`](https://github.com/apache/spark/commit/0d81aa4dde9108233853cab804998aaaab48f998).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r625562055
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(locationUri: Option[URI], configuration: Configuration) {
+ locationUri.fold() { tablePathUri: URI =>
+ val tablePath = tablePathUri.toString
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r624870218
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,20 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(locationUri: Option[URI], configuration: Configuration) {
+ locationUri.fold() { tablePathUri: URI =>
+ val tablePath = tablePathUri.toString
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
Review comment:
I think you can directly create `Path` from `URI`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830657818
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840495349
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43028/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832430678
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138156/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r625213745
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
##########
@@ -166,6 +166,10 @@ case class CreateDataSourceTableAsSelectCommand(
saveDataIntoTable(
sparkSession, table, table.storage.locationUri, child, SaveMode.Append, tableExists = true)
} else {
+ if (mode != SaveMode.Overwrite) {
+ DataWritingCommand.assertEmptyRootPath(table.storage.locationUri,
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840468792
**[Test build #138508 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138508/testReport)** for PR 32411 at commit [`c3f909e`](https://github.com/apache/spark/commit/c3f909ecaf015d39a6fc759e135ed7c93286b630).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631705687
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/DataWritingCommand.scala
##########
@@ -96,4 +98,23 @@ object DataWritingCommand {
sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
metrics.values.toSeq)
}
+
+ def assertEmptyRootPath(tablePath: URI, saveMode: SaveMode, sparkSession: SparkSession) {
+ if (saveMode != SaveMode.Overwrite &&
+ !sparkSession.sqlContext.conf.allowCreateTableAsSelectNonEmptyLocation) {
+ val filePath = new org.apache.hadoop.fs.Path(tablePath)
+ val fs = filePath.getFileSystem(sparkSession.sparkContext.hadoopConfiguration)
+ if(fs != null && fs.exists(filePath)) {
+ val locStats = fs.getFileStatus(filePath)
+ if(locStats != null && locStats.isDirectory) {
Review comment:
combined 3 if conditions to 1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r633636947
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala
##########
@@ -235,10 +235,10 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSparkSession {
}
test("create table using as select - with overridden max number of buckets") {
- def createTableSql(numBuckets: Int): String =
+ def createTableSql(tablePath: String, numBuckets: Int): String =
s"""
|CREATE TABLE t USING PARQUET
- |OPTIONS (PATH '${path.toURI}')
+ |OPTIONS (PATH '$tablePath')
Review comment:
is this change necessary? it `beforeEach` we create a new temp dir and assign it to `path`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-840637497
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138508/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830691948
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138131/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r626729449
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_PATH =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r631706649
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1604,6 +1604,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val ALLOW_CTAS_NON_EMPTY_LOCATION =
+ buildConf("spark.sql.legacy.ctas.allowNonEmptyLocation")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL][WIP]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830611204
**[Test build #138128 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138128/testReport)** for PR 32411 at commit [`293cab3`](https://github.com/apache/spark/commit/293cab33152994f119e80d6e7496e1139896dd72).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL] CTAS with LOCATION should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r632288726
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
##########
@@ -598,6 +598,37 @@ abstract class SQLQuerySuiteBase extends QueryTest with SQLTestUtils with TestHi
}
}
+ test("SPARK-28551: CTAS Hive Table should be with non-existent or empty location") {
+ def executeCTASWithNonEmptyLocation(tempLocation: String) {
+ sql(s"CREATE TABLE ctas1(id string) stored as rcfile LOCATION '$tempLocation/ctas1'")
+ sql("INSERT INTO TABLE ctas1 SELECT 'A' ")
+ sql(s"""CREATE TABLE ctas_with_existing_location stored as rcfile LOCATION
+ |'$tempLocation' AS SELECT key k, value FROM src ORDER BY k, value""".stripMargin)
+ }
+
+ Seq("false", "true").foreach { convertCTASFlag =>
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-830553307
**[Test build #138124 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138124/testReport)** for PR 32411 at commit [`7e996d6`](https://github.com/apache/spark/commit/7e996d6b6e92c5de4697d567e47a2e987551829c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32411: [SPARK-28551][SQL]CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32411:
URL: https://github.com/apache/spark/pull/32411#issuecomment-832891227
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42700/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] vinodkc commented on a change in pull request #32411: [SPARK-28551][SQL]In CTAS with LOCATION , should not allow to a non-empty directory.
Posted by GitBox <gi...@apache.org>.
vinodkc commented on a change in pull request #32411:
URL: https://github.com/apache/spark/pull/32411#discussion_r626730085
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -3707,6 +3716,8 @@ class SQLConf extends Serializable with Logging {
def allowStarWithSingleTableIdentifierInCount: Boolean =
getConf(SQLConf.ALLOW_STAR_WITH_SINGLE_TABLE_IDENTIFIER_IN_COUNT)
+ def allowCreateTableAsSelectNonEmptyDirPath: Boolean = getConf(SQLConf.ALLOW_CTAS_NON_EMPTY_PATH)
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org