yihua commented on code in PR #19914:
URL: https://github.com/apache/hudi/pull/19914#discussion_r4203264897
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderFunction.scala:
##########
@@ -145,6 +148,11 @@ private[parquet] class
HoodieFileGroupReaderFunction(baseFileReader: Broadcast[S
val fileGroupName = FSUtils.getFileIdFromFilePath(sparkAdapter
.getSparkPartitionedFileUtils.getPathFromPartitionedFile(file))
fileSliceMapping.getSlice(fileGroupName) match {
+ case Some(fileSlice) if state.readBaseOnlySlicesDirectly &&
!fileSlice.getLogFiles.findAny().isPresent =>
Review Comment:
I don't think this case can be reached: the only place a
`HoodiePartitionFileSliceMapping` is built is
`PartitionDirectoryConverter.convertFileSliceToPartitionDirectory`, which
embeds a slice only when it has log files or a bootstrap base, so a
base-file-only COW slice already lands in `case _` -> `readBaseFile` on master.
#20115 (merged yesterday, already in this PR's base) reworked that converter
into `convertFileSlicesToPartitionDirectories` and documents exactly this:
base-file-only slices get no mapping and share one directory per partition,
while only slices read as a whole carry the mapping. Could you drop this branch
and the `readBaseOnlySlicesDirectly` state, and update the title/description so
the change is described as vectorized reading + splitting for COW incremental?
Since the measured path has changed twice since the numbers were taken, it
would also be good to re-run the benchmark on the current commit.
--
This is an automated message from the 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]
