yihua commented on code in PR #19914:
URL: https://github.com/apache/hudi/pull/19914#discussion_r4203449099
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderFunction.scala:
##########
@@ -250,7 +260,7 @@ private[parquet] class
HoodieFileGroupReaderFunction(baseFileReader: Broadcast[S
//and we pass an empty schema for the partition schema
val iter = parquetFileReader.read(modifiedFile, readSchema, new
StructType(), state.internalSchemaOpt, filters, conf,
state.tableSchemaAsMessageType)
- projectIfNeeded(iter, readSchema, schemas.outputSchema)
+ projectIfNeeded(applyRequiredFilters(iter, readSchema, requiredFilters),
readSchema, schemas.outputSchema)
Review Comment:
non-blocking: The predicate is now evaluated in all three branches here, but
the incremental suites only reach the first one on COW; the tests that read
partition columns from the file (timestamp/custom key generators,
extract.partition.values.from.path=false) are all MOR, where the reader is
still row-based. Could we add a COW incremental case with a timestamp-based key
generator, so the vectorized path is covered where the read schema carries
mandatory partition fields and the predicate has to resolve against it?
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -207,10 +210,18 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath:
String,
} else {
throw new HoodieNotSupportedException("Unsupported file format: " +
hoodieFileFormat)
}
- supportVectorizedRead = !isIncremental && !isBootstrap && supportBatch
- supportReturningBatch = !isMOR && supportVectorizedRead
+ // MOR incremental embeds file slices that may contain log files
requiring row-level
+ // merging, and CDC rows are built by a row-based iterator that reads
base files through
+ // the same reader, so vectorized reading must be disabled for both. All
other combinations
+ // (COW snapshot, COW incremental, MOR snapshot) either have no log
merging or handle it via
+ // a separate non-vectorized fileGroupBaseFileReader while the base file
reader stays vectorized.
+ supportVectorizedRead = !(isIncremental && (isMOR || isCDC)) &&
!isBootstrap && supportBatch
Review Comment:
non-blocking: This also makes COW incremental reads vectorized and
splittable for ORC and multi-format tables, and I could not find an incremental
query over ORC base files anywhere in the suites (the ORC scenarios in
TestHoodieSparkSqlWriter are snapshot only). Could we add an ORC variant of the
incremental ranges test, or parameterize the existing one by base file format,
so that cell is covered as well?
--
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]
