hudi-agent commented on code in PR #19822:
URL: https://github.com/apache/hudi/pull/19822#discussion_r4079915758
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLegacyParquetReadPath.scala:
##########
@@ -301,6 +306,39 @@ class TestLegacyParquetReadPath extends
HoodieSparkClientTestBase with ScalaAsse
}
}
+ @Test
+ def testBroadcastJoinHonorsPlanTimeBatchingDecision(): Unit = {
Review Comment:
🤖 Does this test actually fail without the fix on the default (Spark 3.5)
profile? Spark35/34's `getReturningBatch` already routes through
`supportsColumnar`, which checks `conf.wholeStageEnabled`, so the pre-fix code
should also return false here — only Spark 3.3's override (`supportBatch`,
which in 3.3 is just `isBatchReadSupportedForSchema`) has the gap. If so, the
regression would only be guarded on the spark3.3 CI profile.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark3-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/Spark3LegacyHoodieParquetFileFormat.scala:
##########
@@ -171,7 +171,11 @@ abstract class
Spark3LegacyHoodieParquetFileFormat(shouldAppendPartitionValues:
val int96RebaseModeInRead = parquetOptions.int96RebaseModeInRead
val timeZoneId = Option(sqlConf.sessionLocalTimeZone)
// Whole stage codegen (PhysicalRDD) is able to deal with batches directly.
- val returningBatch = getReturningBatch(sparkSession, resultSchema)
+ // Respect the plan-time OPTION_RETURNING_BATCH decision when present,
instead of recomputing it here.
+ val returningBatch = enableVectorizedReader &&
Review Comment:
🤖 The Spark 4.0/4.1/4.2 legacy formats are standalone copies that still
recompute `returningBatch` from `supportsColumnar(sparkSession, resultSchema)`
and ignore the option. They're less exposed (their `supportsColumnar` includes
the whole-stage check), but `isTooManyFields` there is evaluated over
`partitionSchema ++ requiredSchema` rather than the plan's output schema, so
they can still disagree with the plan. Worth applying the same fix there?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
