deepakpanda93 commented on code in PR #19471:
URL: https://github.com/apache/hudi/pull/19471#discussion_r3712043197
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestPartitionBucketIndexSupport.scala:
##########
@@ -173,32 +172,31 @@ class TestPartitionBucketIndexSupport extends
TestBucketIndexSupport {
def exprFilePathAnswerCheck(bucketIndexSupport: PartitionBucketIndexSupport,
exprRaw: String, expectResult: Set[String],
allFileStatus: Set[String]): Unit = {
- if (!HoodieSparkUtils.gteqSpark4_0) { // TODO (HUDI-9403)
- val resolveExpr = HoodieCatalystExpressionUtils.resolveExpr(spark,
exprRaw, structSchema)
- val optimizerPlan =
spark.sessionState.optimizer.execute(DummyExpressionHolder(Seq(resolveExpr)))
- val optimizerExpr =
optimizerPlan.asInstanceOf[DummyExpressionHolder].exprs.head
-
- // split input files into different partitions
- val partitionPath1 = DEFAULT_PARTITION_PATH(0)
- val allFileSlices1: Seq[FileSlice] = allFileStatus.slice(0,
3).map(fileName => {
- val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
- slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
- slice
- }).toSeq
-
- val partitionPath2 = DEFAULT_PARTITION_PATH(1)
- val allFileSlices2: Seq[FileSlice] = allFileStatus.slice(3,
5).map(fileName => {
- val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
- slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
- slice
- }).toSeq
-
- val input = Seq((Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath1, Array())),
allFileSlices1),
- (Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath2, Array())),
allFileSlices2))
- val candidate = bucketIndexSupport.computeCandidateFileNames(fileIndex,
splitConjunctivePredicates(optimizerExpr),
- Seq(), input, false)
-
- assert(candidate.get.equals(expectResult))
- }
+ val resolveExpr = HoodieCatalystExpressionUtils.resolveExpr(spark,
exprRaw, structSchema)
+ val dummyExpressionHolder = HoodieDummyExpressionHolder(Seq(resolveExpr),
resolveExpr.references.toSeq)
+ val optimizerPlan =
spark.sessionState.optimizer.execute(dummyExpressionHolder)
+ val optimizerExpr =
optimizerPlan.asInstanceOf[HoodieDummyExpressionHolder].exprs.head
+
+ // split input files into different partitions
+ val partitionPath1 = DEFAULT_PARTITION_PATH(0)
+ val allFileSlices1: Seq[FileSlice] = allFileStatus.slice(0,
3).map(fileName => {
+ val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
+ slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
+ slice
+ }).toSeq
+
+ val partitionPath2 = DEFAULT_PARTITION_PATH(1)
+ val allFileSlices2: Seq[FileSlice] = allFileStatus.slice(3,
5).map(fileName => {
+ val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
+ slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
+ slice
+ }).toSeq
+
+ val input = Seq((Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath1, Array())),
allFileSlices1),
Review Comment:
Good catch, fixed — it is now built with `partitionPath2`.
Your read on it being pre-existing is right, and it was inert:
`SparkBaseIndexSupport.getPrunedPartitionsAndFileNames` takes the partition
from the tuple's `PartitionPath`, never from `FileSlice.getPartitionPath`, so
the expected candidate sets were unaffected. Fixed anyway since the line was
already in the diff.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestPartitionBucketIndexSupport.scala:
##########
@@ -173,32 +172,31 @@ class TestPartitionBucketIndexSupport extends
TestBucketIndexSupport {
def exprFilePathAnswerCheck(bucketIndexSupport: PartitionBucketIndexSupport,
exprRaw: String, expectResult: Set[String],
allFileStatus: Set[String]): Unit = {
- if (!HoodieSparkUtils.gteqSpark4_0) { // TODO (HUDI-9403)
- val resolveExpr = HoodieCatalystExpressionUtils.resolveExpr(spark,
exprRaw, structSchema)
- val optimizerPlan =
spark.sessionState.optimizer.execute(DummyExpressionHolder(Seq(resolveExpr)))
- val optimizerExpr =
optimizerPlan.asInstanceOf[DummyExpressionHolder].exprs.head
-
- // split input files into different partitions
- val partitionPath1 = DEFAULT_PARTITION_PATH(0)
- val allFileSlices1: Seq[FileSlice] = allFileStatus.slice(0,
3).map(fileName => {
- val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
- slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
- slice
- }).toSeq
-
- val partitionPath2 = DEFAULT_PARTITION_PATH(1)
- val allFileSlices2: Seq[FileSlice] = allFileStatus.slice(3,
5).map(fileName => {
- val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
- slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
- slice
- }).toSeq
-
- val input = Seq((Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath1, Array())),
allFileSlices1),
- (Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath2, Array())),
allFileSlices2))
- val candidate = bucketIndexSupport.computeCandidateFileNames(fileIndex,
splitConjunctivePredicates(optimizerExpr),
- Seq(), input, false)
-
- assert(candidate.get.equals(expectResult))
- }
+ val resolveExpr = HoodieCatalystExpressionUtils.resolveExpr(spark,
exprRaw, structSchema)
+ val dummyExpressionHolder = HoodieDummyExpressionHolder(Seq(resolveExpr),
resolveExpr.references.toSeq)
+ val optimizerPlan =
spark.sessionState.optimizer.execute(dummyExpressionHolder)
+ val optimizerExpr =
optimizerPlan.asInstanceOf[HoodieDummyExpressionHolder].exprs.head
+
+ // split input files into different partitions
+ val partitionPath1 = DEFAULT_PARTITION_PATH(0)
+ val allFileSlices1: Seq[FileSlice] = allFileStatus.slice(0,
3).map(fileName => {
+ val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
+ slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
+ slice
+ }).toSeq
+
+ val partitionPath2 = DEFAULT_PARTITION_PATH(1)
+ val allFileSlices2: Seq[FileSlice] = allFileStatus.slice(3,
5).map(fileName => {
+ val slice = new FileSlice(partitionPath1, "00000000000000000",
FSUtils.getFileId(fileName))
+ slice.setBaseFile(new HoodieBaseFile(new StoragePathInfo(new
StoragePath(fileName), 0L, false, 0, 0, 0)))
+ slice
+ }).toSeq
+
+ val input = Seq((Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(partitionPath1, Array())),
allFileSlices1),
Review Comment:
Fixed — `allFileSlices2` now uses `partitionPath2`.
Agreed on both counts: inert today for the reason you gave, and a trap for
anyone adding per-partition coverage later. Which is exactly what the new case
below does, so it would have been the next person's problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
