Skip to content

[SPARK-32481][CORE][SQL] Support truncate table to move data to trash #29387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Udbhav30 wants to merge 7 commits into from
Closed

[SPARK-32481][CORE][SQL] Support truncate table to move data to trash #29387

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
58d957a
[SPARK-32481] Support truncate table to move data to trash
Udbhav30 Aug 8, 2020
81101d9
Handle review comments
Udbhav30 Aug 21, 2020
502dd08
add a warning msg
Udbhav30 Aug 21, 2020
97d2146
add tests and minor fixes
Udbhav30 Aug 22, 2020
a52e9a5
add tests and minor fixes
Udbhav30 Aug 22, 2020
8848b3f
typo fix
Udbhav30 Aug 22, 2020
a2df53b
Review comments fix
Udbhav30 Aug 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Handle review comments
  • Loading branch information
@Udbhav30
Udbhav30 committed Aug 24, 2020
commit 81101d900ebb019a915b26cd52a2119cf85271e9
11 changes: 5 additions & 6 deletions core/src/main/scala/org/apache/spark/util/Utils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,19 +271,18 @@ private[spark] object Utils extends Logging {

/**
* Move data to trash on truncate table given
* spark.sql.truncate.trash.interval is positive
* spark.sql.truncate.trash.enabled is true
*/
def moveToTrashIfEnabled(
fs: FileSystem,
partitionPath: Path,
trashInterval: Int,
isTrashEnabled: Boolean,
hadoopConf: Configuration): Unit = {
if (trashInterval < 0) {
fs.delete(partitionPath, true)
} else {
if (isTrashEnabled && hadoopConf.getInt("fs.trash.interval", 0) > 0) {
logDebug(s"will move data ${partitionPath.toString} to trash")
hadoopConf.setInt("fs.trash.interval", trashInterval)
Trash.moveToAppropriateTrash(fs, partitionPath, hadoopConf)
} else {
fs.delete(partitionPath, true)
}
}

Expand Down
16 changes: 9 additions & 7 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2722,12 +2722,14 @@ object SQLConf {
.booleanConf
.createWithDefault(false)

val TRUNCATE_TRASH_INTERVAL =
buildConf("spark.sql.truncate.trash.interval")
.doc("This Configuration will decide whether move files to trash on truncate table" +
"If -1 files will be deleted without moving to trash")
.intConf
.createWithDefault(-1)
val TRUNCATE_TRASH_ENABLED =
buildConf("spark.sql.truncate.trash.enabled")
Copy link
Member

@HyukjinKwon HyukjinKwon Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick question, do we want to have each configuration for each operation? Looks like #29319 targets similar stuff. Maybe it'd make more sense to have a global configuration.

Copy link
Contributor Author

@Udbhav30 Udbhav30 Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rework on #29319 and make it a global configuration.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. It's too early to make it a global configuration.

.doc("This Configuration will decide whether move files to trash on truncate table given, " +
"'fs.trash.interval' is positive in Hadoop Configuration. " +
"Note that, in Hadoop conf if server side has this configured then the client side " +
"one will be ignored. ")
.booleanConf
.createWithDefault(false)

/**
* Holds information about keys that have been deprecated.
Expand Down Expand Up @@ -3341,7 +3343,7 @@ class SQLConf extends Serializable with Logging {

def legacyPathOptionBehavior: Boolean = getConf(SQLConf.LEGACY_PATH_OPTION_BEHAVIOR)

def truncateTrashInterval: Int = getConf(SQLConf.TRUNCATE_TRASH_INTERVAL)
def truncateTrashEnabled: Boolean = getConf(SQLConf.TRUNCATE_TRASH_ENABLED)

/** ********************** SQLConf functionality methods ************ */

Expand Down
4 changes: 2 additions & 2 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ case class TruncateTableCommand(
}
val hadoopConf = spark.sessionState.newHadoopConf()
val ignorePermissionAcl = SQLConf.get.truncateTableIgnorePermissionAcl
val trashInterval = SQLConf.get.truncateTrashInterval
val isTrashEnabled = SQLConf.get.truncateTrashEnabled
locations.foreach { location =>
if (location.isDefined) {
val path = new Path(location.get)
Expand All @@ -515,7 +515,7 @@ case class TruncateTableCommand(
}
}

Utils.moveToTrashIfEnabled(fs, path, trashInterval, hadoopConf)
Utils.moveToTrashIfEnabled(fs, path, isTrashEnabled, hadoopConf)

// We should keep original permission/acl of the path.
// For owner/group, only super-user can set it, for example on HDFS. Because
Expand Down