yihua opened a new pull request, #19341:
URL: https://github.com/apache/hudi/pull/19341

   ### Describe the issue this Pull Request addresses
   
   The Spark 3.3, 3.4 and 3.5 legacy Parquet file formats
   (`Spark33LegacyHoodieParquetFileFormat`, 
`Spark34LegacyHoodieParquetFileFormat`,
   `Spark35LegacyHoodieParquetFileFormat`) are roughly 90% identical copies of 
each other,
   each ~450 lines. Keeping three near-duplicate readers in sync is 
error-prone: a fix to the
   read path has to be applied three times and can easily drift between 
versions.
   
   ### Summary and Changelog
   
   Extracts the shared logic into a new abstract base,
   `Spark3LegacyHoodieParquetFileFormat`, placed in the shared 
`hudi-spark3-common` module
   (package `org.apache.spark.sql.execution.datasources.parquet`). All three 
version modules
   already depend on `hudi-spark3-common`, and `hudi-spark3-common` is rebuilt 
against each
   Spark 3.x version on its CI leg, so the base compiles unchanged against 3.3, 
3.4 and 3.5.
   
   The base holds `buildReaderWithPartitionValues` and the companion-object 
helpers
   (`pruneInternalSchema`, `rebuildFilterFromParquet`) verbatim. Every 
expression that relies
   on a version-specific Spark API is delegated to a `protected` hook that each 
concrete
   subclass overrides:
   
   - `toAttributes(structType)`: Spark 3.3/3.4 use `StructType.toAttributes`; 
Spark 3.5 uses
     `DataTypeUtils.toAttributes`.
   - `getFilePath(file)`: Spark 3.3 uses `new Path(new URI(file.filePath))`; 
Spark 3.4+ use
     `file.filePath.toPath`.
   - `isVectorizedReaderEnabled(sparkSession, resultSchema)`: Spark 3.3 uses
     `ParquetUtils.isBatchReadSupportedForSchema`; Spark 3.4+ use 
`supportBatch`.
   - `getPushDownStringPredicate(sqlConf)`: renamed from 
`parquetFilterPushDownStringStartWith`
     (3.3) to `parquetFilterPushDownStringPredicate` (3.4+).
   - `getReturningBatch(sparkSession, resultSchema)`: Spark 3.3 uses 
`supportBatch`; Spark 3.4+
     gate on `parquetVectorizedReaderEnabled` and `supportsColumnar` (with 
`WholeStageCodegenExec`).
   - `setParquetTimeConfs(hadoopConf, sparkSession)`: the nanos-as-long and
     infer-timestamp-NTZ conf keys differ between 3.3 and 3.4+.
   
   Each version module keeps a small concrete subclass
   `SparkXXLegacyHoodieParquetFileFormat(appendPartitionValues: Boolean)` that 
extends the base
   and overrides only its hooks (the 3.4/3.5 subclasses additionally keep the 
`supportsColumnar`
   helper they need). The `SparkXXAdapter.createLegacyHoodieParquetFileFormat` 
call sites are
   unchanged and still instantiate `new 
SparkXXLegacyHoodieParquetFileFormat(...)`.
   
   Net effect: the three readers shrink from ~1377 lines total to ~210 lines of 
thin subclasses,
   with ~480 lines of shared logic living once in the base (a net reduction of 
~687 lines). No
   code was copied from elsewhere; the shared body is the pre-existing reader 
logic moved as-is.
   
   The `ParquetFilters` constructor argument list and the 
`InternalSchemaCache.getInternalSchemaByVersionId`
   call are identical across 3.3/3.4/3.5, so they stay in the base without a 
hook.
   
   ### Impact
   
   No user-facing or public API change. This is an internal refactor of the 
legacy Parquet read
   path; behavior is preserved byte-for-byte across all three Spark 3.x 
versions.
   
   The Spark 4.x family (`hudi-spark4-common` serving 4.0/4.1/4.2) is 
intentionally left out of
   scope: Spark 4.0 differs by ~400 lines and uses 4.1+ Parquet APIs, so a 
shared 4.x base would
   not compile on the 4.0 leg. Those copies are untouched.
   
   ### Risk Level
   
   medium
   
   The change moves a hot read path shared across all Spark 3.x deployments. To 
mitigate:
   
   - The shared body is transcribed unchanged from the existing readers, and 
every
     version-specific delta was identified by an exhaustive line-by-line diff 
of the three
     originals and turned into an overridable hook; the base references no 
version-specific
     symbol directly.
   - 
`hudi-spark/src/test/scala/org/apache/hudi/functional/TestLegacyParquetReadPath.scala`
     (already on master) exercises this read path and runs on each version leg, 
acting as the
     safety net for behavior preservation.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [ ] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [ ] Enough context is provided in the sections above
   - [ ] Adequate tests were added if applicable
   


-- 
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]

Reply via email to