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 2019/12/19 22:12:44 UTC

[GitHub] [spark] viirya opened a new pull request #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

viirya opened a new pull request #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956
 
 
   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   
   This patch proposes to preserve few attributes including owner/group/permission/acls of paths when truncate table/partition.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   When Spark SQL truncates table, it deletes the paths of table/partitions, then re-create new ones. If some attributes such as permission/acls are set on the paths, the attributes will be deleted.
   
   We should preserve the permission/acls if possible.
   
   ### Does this PR introduce any user-facing change?
   <!--
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If no, write 'No'.
   -->
   
   Yes. When truncate table/partition, Spark will keep attributes including owner/group/permission/acls of paths.
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   
   Unit test.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363163389
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   Makes sense to me, I don't think we need a config. Shall we also fail if the FS can't set permission?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572707159
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830388
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21224/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569384825
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115875/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362653500
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   Now I retain permission and ACLs. I was doing `fs.setOwner` in first commit for retaining path owner and group. But `fs.setOwner` throws exception because it is only doable by super user.
   
   In Hive code, since it is code running the metastore server, I think it is running with enough permission to set owner/group. For Spark, we may not running it with super user.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572803234
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116413/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213066
 
 
   **[Test build #116181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116181/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572803232
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358374
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20665/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569377366
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832343
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572881463
 
 
   **[Test build #116436 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116436/testReport)** for PR 26956 at commit [`ead4ce4`](https://github.com/apache/spark/commit/ead4ce426a4dc7ef52823ad00e2975e2780c6d22).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569341436
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571044223
 
 
   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116136/
   Test FAILed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362625771
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   For Hive, I try to trace the code path for TRUNCATE TABLE:
   
   1. HiveMetaStore. truncateTableInternal:
   
   Get the locations of table/partitions and call Warehouse.deleteDir for each.
   
   https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L3109
   
   2. Warehouse.deleteDir delegates to MetaStoreFS.deleteDir.
   3. HiveMetaStoreFsImpl (implements MetaStoreFS) delegates to FileUtils.moveToTrash which calls `FileSystem.delete(f, true)`. 
   
   In `HiveMetaStore.truncateTableInternal`, after the location is deleted, new directory is created and previous file status including permissions, group, and ACLs are set to new directory: 
   
   https://github.com/apache/hive/blob/35f86c749cefc2a9972a991deed78a1c3719093d/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/HdfsUtils.java#L288
   
   
   
   
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569341165
 
 
   **[Test build #115869 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115869/testReport)** for PR 26956 at commit [`60134c1`](https://github.com/apache/spark/commit/60134c15bf76e209d5dd44cc8e4fa7c31318d325).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class FakeLocalFsFileSystem extends RawLocalFileSystem `

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572882680
 
 
   **[Test build #116437 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116437/testReport)** for PR 26956 at commit [`8e429a3`](https://github.com/apache/spark/commit/8e429a364fc76573b7ee3637ccaaeef8269700d0).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832343
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569384825
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115875/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830052
 
 
   **[Test build #116436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116436/testReport)** for PR 26956 at commit [`ead4ce4`](https://github.com/apache/spark/commit/ead4ce426a4dc7ef52823ad00e2975e2780c6d22).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572341489
 
 
   **[Test build #116332 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116332/testReport)** for PR 26956 at commit [`ab69638`](https://github.com/apache/spark/commit/ab69638c6e657d699fa5650342eb0dccddad297d).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572733976
 
 
   Also, cc @srowen and @vanzin since this is a security issue.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213709
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569384824
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572709728
 
 
   **[Test build #116413 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116413/testReport)** for PR 26956 at commit [`00a2bc8`](https://github.com/apache/spark/commit/00a2bc89231d9e3de1734e3ed7a6e76656d26bcd).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569384744
 
 
   **[Test build #115875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115875/testReport)** for PR 26956 at commit [`2c3f1fb`](https://github.com/apache/spark/commit/2c3f1fb2f2d7150e99a7beda572c09f23cdc0eef).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569384824
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358374
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20665/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363177907
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   ```
   case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
                   log.warn(s"Can not set original permission $permission back to " +
                     s"the created path: $path. Exception: ${e.getMessage}")
   ```
   it's just a warning

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569341438
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115869/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572733533
 
 
   I have one question, @viirya .
   This PR raises exception because this is a security issue, however SPARK-30312 is registered as `Improvement` for 3.0.0 only. Do you think this security issue is not important enough for back porting? As a release manager of 2.4.5, I guess we need this patch at 2.4.5 release with a fallback configuration.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573341193
 
 
   Ur, @viirya . Our Jenkins with Hadoop-2.7/Hive 1.2 job seems to fail. Could you take a look?
   - https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-2.7-hive-1.2/
   
   Also, @shaneknapp , does our system have different `umask`?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572339912
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364526122
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   > Ur, `AnalysisException` looks weird a little. This is not `Analysis` error.
   
   Maybe `SparkException` or `RuntimeException`?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327956
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364515939
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   > Ur, `AnalysisException` looks weird a little. This is not `Analysis` error.
   
   Ah, this follows throwing AnalysisException at the outside try..catch block. We can change this.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572339912
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213709
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571302669
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116181/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362700789
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   the presto way seems safer 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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572729174
 
 
   Thank you for updating, @viirya .

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569976325
 
 
   ping @cloud-fan @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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364516976
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
 
 Review comment:
   Thanks! Use try...catch instead now.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832352
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21225/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358181
 
 
   **[Test build #115873 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115873/testReport)** for PR 26956 at commit [`1df437b`](https://github.com/apache/spark/commit/1df437b02875b5d31438331f4002954aba8ca5ba).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571302669
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116181/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033729
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571302657
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572803234
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116413/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569377366
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304971
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20661/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362392017
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   not familiar with Hadoop FS APIs, but do we have something like `rm -rf tablePath/*`?. My point is, if we don't delete the parent folder, then we don't have this problem at 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364513115
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
 
 Review comment:
   @cloud-fan @HyukjinKwon By using try catch, I'm not sure what we should do for exception during getPermission/getAcls. I think we don't/can't do anything?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572883211
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116437/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362675581
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   In Presto, looks like it takes the approach by listing all files/directories under the path to delete (i.e., `rm -rf tablePath/*`), and recursively deleting them:
   
   https://github.com/prestodb/presto/blob/9c21aaa659f9d4927fe12bec4b8015c8cbf4722e/presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/SemiTransactionalHiveMetastore.java#L485
   
   https://github.com/prestodb/presto/blob/9c21aaa659f9d4927fe12bec4b8015c8cbf4722e/presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/SemiTransactionalHiveMetastore.java#L1796

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304658
 
 
   **[Test build #115869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115869/testReport)** for PR 26956 at commit [`60134c1`](https://github.com/apache/spark/commit/60134c15bf76e209d5dd44cc8e4fa7c31318d325).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358372
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304146
 
 
   retest this please

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514898
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
+                  s"Failed to set original permission $permission back to " +
+                    s"the created path: $path. Exception: ${e.getMessage}")
+            }
+          }
+          optAcls.foreach { acls =>
+            try {
+              fs.setAcl(path, acls)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   ditto.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573347537
 
 
   @dongjoon-hyun Thanks for pinging me. Opened a followup at #27175.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572381920
 
 
   **[Test build #116329 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116329/testReport)** for PR 26956 at commit [`7b29f2f`](https://github.com/apache/spark/commit/7b29f2f6a459d6ce4717d29973bafb5647aff123).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572397903
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116332/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362394495
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   In the [FileSystem API](https://hadoop.apache.org/docs/current/api/org/apache/hadoop/fs/FileSystem.html), I didn't find one like that. 
   
   But I just search again, there is [FileUtil API ](https://hadoop.apache.org/docs/current/api/org/apache/hadoop/fs/FileUtil.html#fullyDeleteContents-java.io.File-) that seems working like that.
   
   I will test tomorrow to see if it work well to keep permission/acl in a Spark cluster like current approach.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033385
 
 
   **[Test build #116136 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116136/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569377264
 
 
   **[Test build #115873 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115873/testReport)** for PR 26956 at commit [`1df437b`](https://github.com/apache/spark/commit/1df437b02875b5d31438331f4002954aba8ca5ba).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571211246
 
 
   retest this please

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327586
 
 
   **[Test build #116329 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116329/testReport)** for PR 26956 at commit [`7b29f2f`](https://github.com/apache/spark/commit/7b29f2f6a459d6ce4717d29973bafb5647aff123).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370722
 
 
   **[Test build #115875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115875/testReport)** for PR 26956 at commit [`2c3f1fb`](https://github.com/apache/spark/commit/2c3f1fb2f2d7150e99a7beda572c09f23cdc0eef).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572707168
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21201/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830381
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572339924
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21121/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363627079
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
 
 Review comment:
   Yeah .. `Try(...)` is actually discouraged .. :-).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363167994
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   We already fail if the FS cannot set permission. No?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832020
 
 
   **[Test build #116437 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116437/testReport)** for PR 26956 at commit [`8e429a3`](https://github.com/apache/spark/commit/8e429a364fc76573b7ee3637ccaaeef8269700d0).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363609490
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
 
 Review comment:
   can we use the normal `try-catch`?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213718
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20974/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573341193
 
 
   Ur, @viirya . Our Jenkins with Hadoop-2.7/Hive 1.2 job seems to fail. Could you take a look?
   - https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-2.7-hive-1.2/

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571044160
 
 
   **[Test build #116136 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116136/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).
    * This patch **fails due to an unknown error code, -9**.
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304968
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572770558
 
 
   Thank you, @viirya and @srowen .
   @viirya . Security issue is similar with correctness issue. We need to fix security issue like correctness issue if we know the issue and how to fix it.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370844
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20667/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514416
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   Ur, `AnalysisException` looks weird a little. This is not `Analysis` error.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033734
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20929/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364516746
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   > Also, `Exception` is too risky behavior change. (@gatorsmile ).
   > I guess `warning` might be enough?
   
   For security reason, cannot set back original Acl/permission seems serious issue so `Exception` is thrown now. `warning` might be ignored and not noticed by users.
   
   I'm ok with a `warning` if you all think it isn't an issue.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327964
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21119/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514871
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   Also, `Exception` is too risky behavior change. (@gatorsmile ).
   I guess `warning` might be enough?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327586
 
 
   **[Test build #116329 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116329/testReport)** for PR 26956 at commit [`7b29f2f`](https://github.com/apache/spark/commit/7b29f2f6a459d6ce4717d29973bafb5647aff123).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304658
 
 
   **[Test build #115869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115869/testReport)** for PR 26956 at commit [`60134c1`](https://github.com/apache/spark/commit/60134c15bf76e209d5dd44cc8e4fa7c31318d325).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327956
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573353773
 
 
   Thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572883206
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572327964
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21119/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572733533
 
 
   I have one question, @viirya .
   This PR raises exception because this is a security issue, however [SPARK-30312](https://issues.apache.org/jira/browse/SPARK-30312) is registered as major `Improvement` for 3.0.0 only. Do you think this security issue is not important enough for back porting? As a release manager of 2.4.5, I guess we need this patch at 2.4.5 release with a fallback configuration.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830388
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21224/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572883211
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116437/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364567926
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   Based on previous discussion around https://github.com/apache/spark/pull/26956#discussion_r363163389, we may not need a config.
   
   `SecurityException` seems ok. cc @cloud-fan 

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572382408
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116329/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572803232
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572382402
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] gatorsmile commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r366680558
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
 ##########
 @@ -1988,6 +1988,14 @@ object SQLConf {
       .booleanConf
       .createWithDefault(false)
 
+  val TRUNCATE_TABLE_IGNORE_PERMISSION_ACL =
+    buildConf("spark.sql.truncateTable.ignorePermissionAcl")
 
 Review comment:
   -> spark.sql.truncateTable.ignorePermissionAcl.enabled?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213066
 
 
   **[Test build #116181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116181/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514061
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
 
 Review comment:
   In this case, I guess `case NonFatal(_) => // do nothing` is okay.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572707159
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363179571
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   Oh, I see. You meant `UnsupportedOperationException` case. Hmm, `setAcl` also has `UnsupportedOperationException` case. I'm not sure if it is possible that we can get permission/ACLs, but `setPermission`/`setAcl` throws `UnsupportedOperationException`.
   
   Logically, following above reason, it should be safer to fail it too if it is `UnsupportedOperationException`.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304971
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20661/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033729
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571044223
 
 
   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116136/
   Test FAILed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572761320
 
 
   @dongjoon-hyun Thanks for notifying me this. This looks worth back-porting, though it is not a regression as we never preserve permission/ACLs before, so my opinion is not strong too.
   
   @srowen I think it depends on case by case. Internally we have seen performance issue on sequential RPC calls to namenode (not truncate table, but file commit ops), so my first thought is not to do it again here.
   
   Our Spark users reported that the custom ACLs they set were lost after truncating table. There could be non default ACLs setting on the table path we don't like to lose it just because truncating table.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569341436
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370842
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r363627305
 
 

 ##########
 File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
 ##########
 @@ -1981,6 +1982,48 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
     }
   }
 
+  test("truncate table - keep acl/permission") {
 
 Review comment:
   Maybe it's best to add JIRA id `SPARK-30312: ...`

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572707168
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21201/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362708973
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   I was concerned with the presto way on performance regression. For a table which has many files/directories, deleting them one by one could be a bottleneck.
   
   If we add a config for retaining permission/ACLs when truncating table? Users can choose to disable it and directly delete top directory (current behavior) without retaining permission/ACLs.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572881819
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573180954
 
 
   Could you make a backporting PR to `branch-2.4`, @viirya ?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830052
 
 
   **[Test build #116436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116436/testReport)** for PR 26956 at commit [`ead4ce4`](https://github.com/apache/spark/commit/ead4ce426a4dc7ef52823ad00e2975e2780c6d22).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572883206
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572830381
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364607773
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   SecurityException sounds good

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571302657
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572339924
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21121/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572881825
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116436/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362625771
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   For Hive, I try to trace the code path for TRUNCATE TABLE:
   
   1. HiveMetaStore. truncateTableInternal:
   
   Get the locations of table/partitions and call Warehouse.deleteDir for each.
   
   https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L3109
   
   2. Warehouse.deleteDir delegates to MetaStoreFS.deleteDir.
   3. HiveMetaStoreFsImpl (implements MetaStoreFS) delegates to FileUtils.moveToTrash which calls `FileSystem.delete(f, true)`. 
   
   In `HiveMetaStore.truncateTableInternal`, after the location is deleted, new directory is created and previous file status including permissions, group, and ACLs are set to new 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832020
 
 
   **[Test build #116437 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116437/testReport)** for PR 26956 at commit [`8e429a3`](https://github.com/apache/spark/commit/8e429a364fc76573b7ee3637ccaaeef8269700d0).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572733533
 
 
   I have one question, @viirya .
   This PR raises exception because this is a security issue, however [SPARK-30312](https://issues.apache.org/jira/browse/SPARK-30312) is registered as `Improvement` for 3.0.0 only. Do you think this security issue is not important enough for back porting? As a release manager of 2.4.5, I guess we need this patch at 2.4.5 release with a fallback configuration.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033734
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20929/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571213718
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20974/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569341438
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115869/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572802582
 
 
   **[Test build #116413 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116413/testReport)** for PR 26956 at commit [`00a2bc8`](https://github.com/apache/spark/commit/00a2bc89231d9e3de1734e3ed7a6e76656d26bcd).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572341489
 
 
   **[Test build #116332 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116332/testReport)** for PR 26956 at commit [`ab69638`](https://github.com/apache/spark/commit/ab69638c6e657d699fa5650342eb0dccddad297d).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514416
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   Ur, `AnalysisException` sounds weird.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572397895
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun closed pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362400743
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   oh, I think `FileUtil.fullyDeleteContents` only works on local file (java.io.File), not for files on distributed file system like HDFS.
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514122
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
 
 Review comment:
   ditto.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572832352
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/21225/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572397450
 
 
   **[Test build #116332 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116332/testReport)** for PR 26956 at commit [`ab69638`](https://github.com/apache/spark/commit/ab69638c6e657d699fa5650342eb0dccddad297d).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362727331
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   do we have to fail if we can't set the permission back?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362936634
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   Again, we can consider to add one config that users can choose to fail if we cannot preserve original permission/ACLs. For non secure cluster, it can be ignored.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573182773
 
 
   @dongjoon-hyun Sure. Thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569304968
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370722
 
 
   **[Test build #115875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115875/testReport)** for PR 26956 at commit [`2c3f1fb`](https://github.com/apache/spark/commit/2c3f1fb2f2d7150e99a7beda572c09f23cdc0eef).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362881517
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            Try(fs.setPermission(path, permission)) match {
+              case Failure(e) if e.isInstanceOf[UnsupportedOperationException] =>
+                log.warn(s"Can not set original permission $permission back to " +
+                  s"the created path: $path. Exception: ${e.getMessage}")
+              case Failure(e) => throw e
 
 Review comment:
   In internal discussion about this issue, we think it is more dangerous to have wrong permission/ACLs silently on a secure cluster when truncate table.  This failure tells users what is wrong and they could so something manually to fix it. 

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569377367
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115873/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364548336
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   - If this is for security, what about `SecurityException`?
   - If we need to raise an exception, shall we have a fallback configuration?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572881819
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571044219
 
 
   Merged build finished. Test FAILed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572881825
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116436/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572382402
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572709728
 
 
   **[Test build #116413 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116413/testReport)** for PR 26956 at commit [`00a2bc8`](https://github.com/apache/spark/commit/00a2bc89231d9e3de1734e3ed7a6e76656d26bcd).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572382408
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116329/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r366680873
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
 ##########
 @@ -1988,6 +1988,14 @@ object SQLConf {
       .booleanConf
       .createWithDefault(false)
 
+  val TRUNCATE_TABLE_IGNORE_PERMISSION_ACL =
+    buildConf("spark.sql.truncateTable.ignorePermissionAcl")
 
 Review comment:
   Ok. I will create a followup for this. Thanks.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572397895
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572789610
 
 
   @dongjoon-hyun Ok. Then I think adding a fallback configuration here too can make back-porting easier?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569377367
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/115873/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370844
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/20667/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358181
 
 
   **[Test build #115873 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115873/testReport)** for PR 26956 at commit [`1df437b`](https://github.com/apache/spark/commit/1df437b02875b5d31438331f4002954aba8ca5ba).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362544508
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   So I think there is no single API to do `rm -rf tablePath/*` in Hadoop FS, if I haven't missed anything.
   
   We can do `listStatus` and delete all contents in the given directory. But it is inefficient for DFS and is easier to have error.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362392017
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   not familiar with Hadoop FS APIs, but do we have something like `rm -rf tablePath/*`. My point is, if we don't delete the parent folder, then we don't have this problem at 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569370842
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571302080
 
 
   **[Test build #116181 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116181/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).
    * 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-572397903
 
 
   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/116332/
   Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r365108284
 
 

 ##########
 File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
 ##########
 @@ -1981,6 +1982,60 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
     }
   }
 
+  test("SPARK-30312: truncate table - keep acl/permission") {
+    import testImplicits._
+    val ignorePermissionAcl = Seq(true, false)
+
+    ignorePermissionAcl.foreach { ignore =>
+      withSQLConf(
+        "fs.file.impl" -> classOf[FakeLocalFsFileSystem].getName,
+        "fs.file.impl.disable.cache" -> "true",
+        SQLConf.TRUNCATE_TABLE_IGNORE_PERMISSION_ACL.key -> ignore.toString) {
+        withTable("tab1") {
+          sql("CREATE TABLE tab1 (col INT) USING parquet")
+          sql("INSERT INTO tab1 SELECT 1")
+          checkAnswer(spark.table("tab1"), Row(1))
+
+          val tablePath = new Path(spark.sessionState.catalog
+            .getTableMetadata(TableIdentifier("tab1")).storage.locationUri.get)
+
+          val hadoopConf = spark.sessionState.newHadoopConf()
+          val fs = tablePath.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(tablePath);
+
+          fs.setPermission(tablePath, new FsPermission("777"))
+          assert(fileStatus.getPermission().toString() == "rwxrwxrwx")
+
+          // Set ACL to table path.
+          val customAcl = new java.util.ArrayList[AclEntry]()
+          customAcl.add(new AclEntry.Builder()
+            .setType(AclEntryType.USER)
+            .setScope(AclEntryScope.ACCESS)
+            .setPermission(FsAction.READ).build())
+          fs.setAcl(tablePath, customAcl)
+          assert(fs.getAclStatus(tablePath).getEntries().get(0) == customAcl.get(0))
+
+          sql("TRUNCATE TABLE tab1")
+          assert(spark.table("tab1").collect().isEmpty)
+
+          val fileStatus2 = fs.getFileStatus(tablePath)
+          if (ignore) {
+            assert(fileStatus2.getPermission().toString() == "rwxr-xr-x")
 
 Review comment:
   Good point! Let me update it.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571033385
 
 
   **[Test build #116136 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/116136/testReport)** for PR 26956 at commit [`39fe234`](https://github.com/apache/spark/commit/39fe2343dbf332f12be95de2e845a8e197a87f73).

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r364514416
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +500,48 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          var optPermission: Option[FsPermission] = None
+          try {
+            optPermission = Some(fileStatus.getPermission())
+          } catch {
+            case NonFatal(e) =>
+          }
+
+          var optAcls: Option[java.util.List[AclEntry]] = None
+          try {
+            optAcls = Some(fs.getAclStatus(path).getEntries)
+          } catch {
+            case NonFatal(e) =>
+          }
+
           fs.delete(path, true)
+
+          // We should keep original permission/acl of the path.
+          // For owner/group, only super-user can set it, for example on HDFS. Because
+          // current user can delete the path, we assume the user/group is correct or not an issue.
           fs.mkdirs(path)
+          optPermission.foreach { permission =>
+            try {
+              fs.setPermission(path, permission)
+            } catch {
+              case NonFatal(e) =>
+                throw new AnalysisException(
 
 Review comment:
   Ur, `AnalysisException` looks weird.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-573183050
 
 
   Thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r362575589
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
 ##########
 @@ -499,8 +499,35 @@ case class TruncateTableCommand(
         val path = new Path(location.get)
         try {
           val fs = path.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(path)
+          // Not all fs impl. support these APIs.
+          val optPermission = Try(fileStatus.getPermission())
+          val optAcls = Try(fs.getAclStatus(path).getEntries)
+
           fs.delete(path, true)
 
 Review comment:
   Do you know how Hive/Presto implement TRUNCATE TABLE? Are there other file attributes we need to retain?

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #26956: [SPARK-30312][SQL] Preserve path attributes including permission when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-569358372
 
 
   Merged build finished. Test PASSed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#issuecomment-571044219
 
 
   Merged build finished. Test FAILed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #26956: [SPARK-30312][SQL] Preserve path permission and acl when truncate table
URL: https://github.com/apache/spark/pull/26956#discussion_r365086989
 
 

 ##########
 File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
 ##########
 @@ -1981,6 +1982,60 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
     }
   }
 
+  test("SPARK-30312: truncate table - keep acl/permission") {
+    import testImplicits._
+    val ignorePermissionAcl = Seq(true, false)
+
+    ignorePermissionAcl.foreach { ignore =>
+      withSQLConf(
+        "fs.file.impl" -> classOf[FakeLocalFsFileSystem].getName,
+        "fs.file.impl.disable.cache" -> "true",
+        SQLConf.TRUNCATE_TABLE_IGNORE_PERMISSION_ACL.key -> ignore.toString) {
+        withTable("tab1") {
+          sql("CREATE TABLE tab1 (col INT) USING parquet")
+          sql("INSERT INTO tab1 SELECT 1")
+          checkAnswer(spark.table("tab1"), Row(1))
+
+          val tablePath = new Path(spark.sessionState.catalog
+            .getTableMetadata(TableIdentifier("tab1")).storage.locationUri.get)
+
+          val hadoopConf = spark.sessionState.newHadoopConf()
+          val fs = tablePath.getFileSystem(hadoopConf)
+          val fileStatus = fs.getFileStatus(tablePath);
+
+          fs.setPermission(tablePath, new FsPermission("777"))
+          assert(fileStatus.getPermission().toString() == "rwxrwxrwx")
+
+          // Set ACL to table path.
+          val customAcl = new java.util.ArrayList[AclEntry]()
+          customAcl.add(new AclEntry.Builder()
+            .setType(AclEntryType.USER)
+            .setScope(AclEntryScope.ACCESS)
+            .setPermission(FsAction.READ).build())
+          fs.setAcl(tablePath, customAcl)
+          assert(fs.getAclStatus(tablePath).getEntries().get(0) == customAcl.get(0))
+
+          sql("TRUNCATE TABLE tab1")
+          assert(spark.table("tab1").collect().isEmpty)
+
+          val fileStatus2 = fs.getFileStatus(tablePath)
+          if (ignore) {
+            assert(fileStatus2.getPermission().toString() == "rwxr-xr-x")
 
 Review comment:
   Thank you for updating, @viirya .  This test case may fail on the system where the default umask is not 0022. Can we have a more robust way?
   
   ```
   ~$ umask
   0022
   ~$ mkdir d0022
   ~$ ls -al d0022
   total 0
   drwxr-xr-x   2 dongjoon  staff    64 Jan  9 22:18 .
   drwxr-xr-x@ 87 dongjoon  staff  2784 Jan  9 22:18 ..
   
   ~$ umask 0011
   ~$ mkdir d0011
   ~$ ls -al d0011
   total 0
   drwxrw-rw-   2 dongjoon  staff    64 Jan  9 22:19 .
   drwxr-xr-x@ 88 dongjoon  staff  2816 Jan  9 22:19 ..
   ```

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org