manoj-ragupathy commented on issue #12569:
URL: https://github.com/apache/gluten/issues/12569#issuecomment-5667813471

   I'd like to pick this up. I've completed an initial Spark 4.2.0 bring-up for 
the Velox backend and would be glad to contribute it upstream if the approach 
looks right to the community.
   
   cc @weiting-chen @wangyum @baibaichen — following up on the "gathering 
feedback" note above.
   
   ### Proposed delivery — four sequenced PRs
   
   Mirroring the Spark 4.1 sequence (#11313 → #11347 → #11353 → #11380), which 
maps onto the proposed work list in the description:
   
   | PR | Scope | Size |
   |---|---|---|
   | 1 | Fix compatibility issues introduced by Spark 4.2 (version-agnostic 
only) | 19 files |
   | 2 | `spark-4.2` build profile + `shims/spark42` layer + CI resources | 44 
files |
   | 3 | `gluten-ut/spark42` unit test module + CI UT jobs | ~1850 files |
   | 4 | Additional Spark 4.2 test suites | 4 files |
   
   Splitting this way keeps PR-1 and PR-2 small enough to review before the 
large `gluten-ut` move lands.
   
   ### Verification so far
   
   All four branches build against their target profiles. PR-1 is 
version-agnostic by construction and compile-verified on Spark **3.4, 3.5, 4.0 
and 4.1** (it contains no `spark42` paths), so it should be safe to merge 
independently of the rest.
   
   I should be clear about the limits: this is **compile and build verification 
only**. I have not been able to execute the unit tests locally, so the suites 
in PR-3/PR-4 are unproven at runtime — that needs CI (see blocker below).
   
   Both #12567 and #12556 are already in my base, and I did not redo them.
   
   ### Main compatibility deltas found
   
   Most of the Spark 4.2 delta is absorbed by the shim layer, but not all of it:
   
   - **SPJ redesign** — `KeyGroupedPartitioning` → `KeyedPartitioning`, 
`StoragePartitionJoinParams` and `KeyGroupedPartitionedScan` removed, 
`BatchScanExec` constructor changed, `filteredPartitions` now 
`Seq[Option[InputPartition]]`, runtime filters pushed via 
`PushDownUtils.pushRuntimeFilters`.
   - **`postDriverMetrics`** now takes an explicit `Array[CustomTaskMetric]`.
   - **`SampleExec.seed`** became `Option[Long]`.
   - **`QueryTest`** — 4.2 hoists `checkAnswer` and friends into a new 
`QueryTestBase` trait that `SharedSparkSession` mixes in, which collided with 
`GlutenQueryTest`'s own copies.
   - **Netty 4.2.13** removed `PlatformDependent.allocateDirectNoCleaner`.
   - `CharType`, `AppendDataExec`, `OverwriteByExpressionExec`, 
`BroadcastHashJoinExec` and `DataSourceV2ScanRelation` all gained parameters, 
breaking positional extractor patterns.
   
   **One behavioural degradation I want to flag explicitly for review:** Spark 
4.2 removed `spjParams`, and I found no scan-level equivalent for the 
partially-clustered "common partition values" (that logic moved into 
`EnsureRequirements`/`GroupPartitionsExec`). On 4.2 only, 
`getCommonPartitionValues` conservatively returns `None`, which disables the 
partially-clustered-distribution refinement; the base fully-clustered SPJ path 
is unaffected. Spark 3.4–4.1 behaviour is unchanged. I'd appreciate scrutiny on 
whether that's an acceptable starting point or needs a deeper port.
   
   ### Two things I need help with
   
   1. **The 4.2 UT jobs cannot pass on a code change alone.** 
`-Dspark.test.home=/opt/shims/spark42/spark_home/` is baked into the CI 
container image at image-build time, and `docker_image.yml` only rebuilds on 
the Sunday cron. PR-2 adds the `install-spark-resources.sh 4.2` lines to the 
Dockerfiles, but someone with committer rights would need to 
`workflow_dispatch` an image rebuild before the UT lanes in PR-3 can go green.
   2. **PR-3 must be rebase-merged, not squashed** — it uses the move/copy-back 
commit structure from #11352 so `git log --follow` keeps history on the 
migrated suites. I verified `--follow` still traces through the move into 
pre-existing history.
   
   Also note the 4.2 UT lane drops `-Pdelta`: `delta-spark_4.2`, 
`iceberg-spark-runtime-4.2_2.13` and `hudi-spark4.2-bundle_2.13` do not exist 
on Maven Central yet, so those lanes can only be enabled once artifacts ship.
   
   Happy to adjust the split, ordering, or scope based on what reviewers 
prefer. Could this issue be assigned to me?
   
   Generated-by: GitHub Copilot CLI (Claude Opus 5)
   


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to