Skip to content

Commit

Permalink
[SPARK-34326][CORE][SQL] Fix UTs added in SPARK-31793 depending on th…
Browse files Browse the repository at this point in the history
…e length of temp path

### What changes were proposed in this pull request?

This PR proposes to fix the UTs being added in SPARK-31793, so that all things contributing the length limit are properly accounted.

### Why are the changes needed?

The test `DataSourceScanExecRedactionSuite.SPARK-31793: FileSourceScanExec metadata should contain limited file paths` is failing conditionally, depending on the length of the temp directory.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Modified UTs explain the missing points, which also do the test.

Closes #31435 from HeartSaVioR/SPARK-34326.

Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com>
Signed-off-by: Jungtaek Lim <kabhwan.opensource@gmail.com>
  • Loading branch information
HeartSaVioR committed Feb 2, 2021
1 parent cadca8d commit 6386602
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
6 changes: 6 additions & 0 deletions core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
Expand Up @@ -1308,6 +1308,12 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
assert(Utils.buildLocationMetadata(paths, 10) == "[path0, path1]")
assert(Utils.buildLocationMetadata(paths, 15) == "[path0, path1, path2]")
assert(Utils.buildLocationMetadata(paths, 25) == "[path0, path1, path2, path3]")

// edge-case: we should consider the fact non-path chars including '[' and ", " are accounted
// 1. second path is not added due to the addition of '['
assert(Utils.buildLocationMetadata(paths, 6) == "[path0]")
// 2. third path is not added due to the addition of ", "
assert(Utils.buildLocationMetadata(paths, 13) == "[path0, path1]")
}

test("checkHost supports both IPV4 and IPV6") {
Expand Down
Expand Up @@ -137,9 +137,24 @@ class DataSourceScanExecRedactionSuite extends DataSourceScanRedactionTest {
assert(location.isDefined)
// The location metadata should at least contain one path
assert(location.get.contains(paths.head))
// If the temp path length is larger than 100, the metadata length should not exceed
// twice of the length; otherwise, the metadata length should be controlled within 200.
assert(location.get.length < Math.max(paths.head.length, 100) * 2)

// The location metadata should have bracket wrapping paths
assert(location.get.indexOf('[') > -1)
assert(location.get.indexOf(']') > -1)

// extract paths in location metadata (removing classname, brackets, separators)
val pathsInLocation = location.get.substring(
location.get.indexOf('[') + 1, location.get.indexOf(']')).split(", ").toSeq

// If the temp path length is less than (stop appending threshold - 1), say, 100 - 1 = 99,
// location should include more than one paths. Otherwise location should include only one
// path.
// (Note we apply subtraction with 1 to count start bracket '['.)
if (paths.head.length < 99) {
assert(pathsInLocation.size >= 2)
} else {
assert(pathsInLocation.size == 1)
}
}
}
}
Expand Down

0 comments on commit 6386602

Please sign in to comment.