build: enable Spark SQL tests for released Spark 4.2.0 - #4950
Draft
andygrove wants to merge 21 commits into
Draft
Conversation
Adds dev/diffs/4.2.0-preview4.diff so the Spark SQL test suite can run against Apache Spark 4.2.0-preview4 with Comet enabled, and wires the 4.2 profile into the spark_sql_test workflow matrix. The diff was seeded from 4.1.1.diff and reconciled against v4.2.0-preview4.
Inherited from 4.1.1.diff during reconciliation. The Spark build was trying to resolve comet-spark-spark4.1_2.13 instead of the 4.2 artifact, causing the sql_hive jobs to fail before any tests ran.
Spark 4.2 removes GeographyVal / GeometryVal and replaces the getGeography and getGeometry accessors on SpecializedGetters with a single getBinaryView returning BinaryView. The shim therefore can no longer be shared between 4.1 and 4.2 out of the spark-4.1+ source root: the major, minor-plus and minor shim directories are all added as compile source roots, so a spark-4.2 copy would be a duplicate class rather than an override. Move the trait into per-version spark-4.1 and spark-4.2 copies.
Point spark.version at the released 4.2.0 instead of 4.2.0-preview4. Spark 4.2.0 bundles its own copy of org.apache.datasketches.memory.internal.ResourceImpl inside spark-catalyst, which collides with the transitive datasketches-memory jar and fails the maven-enforcer BanDuplicateClasses rule, so ignore that class. Keep the test-scope Jetty pin at 11.0.26. Spark 4.2.0 ships Jetty 12, but the Iceberg REST catalog test helper needs jetty-servlet, which Jetty 12 replaced with jetty-ee10-servlet.
Reseed the Spark SQL test overrides from 4.1.2.diff against the v4.2.0 tag and drop the 4.2.0-preview4 diff. The notable reconciliation is the test harness refactor: Spark 4.2 shrinks SQLTestUtils to a deprecated empty alias and moves withSQLConf, stripSparkFilter and the test() override into QueryTest, with SharedSparkSession now extending QueryTest directly. Comet's hooks (isCometEnabled, the IgnoreComet skip and the stripSparkFilter Comet cases) move to QueryTest accordingly. isCometEnabled has to qualify classic.SparkSession there, since a bare SparkSession in that package resolves to the unified class.
Regenerated with dev/regenerate-golden-files.sh --spark-version 4.2. The q2, q5, q54 (v1_4) and q5a (v2_7) goldens are pruned: their plans now match the shared fallback under the released 4.2.0, so the divergences seen on 4.2.0-preview4 are gone. q77a (v2_7) is added because Spark 4.2 plans a OneRowRelation into its Union branches. Comet cannot convert that leaf, so the Unions fall back and the aggregates above them lose their Comet partial producer. The approved plan records the degraded plan; see apache#4949.
Add a spark_4_2 job to the ci.yml umbrella calling spark_sql_test_reusable.yml with Spark 4.2.0 on JDK 17, plus the matching spark_4_2 filter in compute-changes.py and output on the changes job. Gate it behind the run-spark-4.2-tests label so it runs on pushes to main but stays off the default PR path while 4.2 support is experimental. Update the user guide for the released 4.2.0 and refresh the new-Spark-version contributor guide, which still described the spark_sql_test.yml matrix that was replaced by the per-version umbrella jobs.
The preflight job only proceeds on a labeled event for known gating labels. Without run-spark-4.2-tests in that allowlist, labelling a PR to request the Spark 4.2 SQL tests skipped the whole pipeline instead of running them.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
andygrove
marked this pull request as draft
July 17, 2026 15:57
Spark 4.2.0 is now in Maven Central, so the spark-4.2 profile no longer needs to target a preview. Point spark.version at 4.2.0 and make the main sources compile against it. - Drop the WIP compile-only comment on the spark-4.2 profile. - Spark 4.2.0 bundles its own copy of org.apache.datasketches.memory.internal.ResourceImpl inside spark-catalyst, colliding with the transitive datasketches-memory jar and failing the BanDuplicateClasses enforcer rule; ignore that class. - Spark 4.2 removes GeographyVal / GeometryVal and replaces getGeography / getGeometry on SpecializedGetters with a single getBinaryView returning BinaryView. CometInternalRowShim can no longer be shared from the spark-4.1+ source root, so split it into per-version spark-4.1 and spark-4.2 copies. - Correct the stale Jetty comment that referenced preview4. This does not wire up the Spark SQL tests for 4.2; it only moves the profile onto the released version and keeps it compiling.
Regenerated with dev/regenerate-golden-files.sh --spark-version 4.2 against the released 4.2.0. The q2, q5, q54 (v1_4) and q5a (v2_7) goldens are pruned because their plans now match the shared fallback on 4.2.0, and q77a (v2_7) is added to record a degraded plan.
Two Spark 4.2.0 changes surfaced by moving the profile off preview4: Iceberg scans job: Spark 4.2.0 turned `connector.catalog.View` from an interface into a class. No Iceberg spark-runtime is published for 4.2, so the build reuses the 4.0 runtime, whose `SparkView implements View` now throws IncompatibleClassChangeError at class-load and aborts the Iceberg suites. Report Iceberg as unavailable on Spark 4.2 in the shared probes so the suites skip. Guard the fuzz suite's beforeAll with an early return (cancelling from beforeAll aborts the suite) and add the missing `assume(icebergAvailable)` to one native-scan test. Expressions job: Spark 4.2 normalizes NaN / -0.0 for array_distinct and the array set operations by wrapping their inputs as `KnownFloatingPointNormalized(ArrayTransform(arr, x -> NormalizeNaNAndZero(x)))`. The serde only handled a scalar `NormalizeNaNAndZero` child and fell back. Since `KnownFloatingPointNormalized` is a runtime no-op tag, serialize any other child directly and let its serde (the ArrayTransform codegen dispatcher) carry the normalization, keeping these operations native.
# Conflicts: # spark/src/main/scala/org/apache/comet/serde/contraintExpressions.scala
CometNativeScan serializes the full data and partition schema, not just the required columns. A table with a GEOMETRY or GEOGRAPHY column (Spark 4.2) has no proto representation for those types, so schema2Proto threw NoSuchElementException during planning instead of falling back. Reject the native scan when any data or partition schema field type cannot be serialized.
…issues Handle new-in-4.2 Spark SQL test suites that assert on Spark-internal execution mechanics Comet replaces wholesale, and repoint deferred assumes at specific issues instead of the closed generic tracker (apache#4142). - SLAM metric suites, segment-tree window suites, and the UnionExec whole-stage-codegen test are marked IgnoreComet / IgnoreCometSuite (apache#4963, apache#4964, apache#4965). - collect_set NaN/-0.0 normalization deferred (apache#4966). - Repoint assume(!isSpark42Plus) guards to specific issues: apache#4967 (ANSI arithmetic), apache#4968 (BloomFilter), apache#4969 (Iceberg REST catalog). - Geometry/geography scan tests now pass via native-scan fallback.
CodeQL flagged ci.yml as not limiting the GITHUB_TOKEN, so it inherits whatever the repository default is. Adds a top-level `permissions: contents: read`, matching the convention already used by codeql.yml and pyarrow_udf_test.yml. A blanket read-only default is not quite safe here, because ci.yml calls six reusable workflows and a callee inherits the caller's permissions and can only narrow them further. Audited all six: docs.yaml commits and pushes the generated site to the asf-site branch, so it needs write; the other five (pr_build_linux, pr_build_macos, pr_benchmark_check, spark_sql_test_reusable, iceberg_spark_test_reusable) only build and test. So the `docs` job raises itself to `contents: write` and nothing else changes. That job already only runs on push-to-main or workflow_dispatch, never on pull requests. Also checked what else might quietly depend on write access: no job in ci.yml or in any callee uses GITHUB_TOKEN, secrets, the gh CLI, git push (other than docs.yaml), containers or packages. The upload/download-artifact steps are all same-run, which uses the Actions runtime token rather than GITHUB_TOKEN, and none pass run-id or github-token, so none need `actions: read`. Verified the file parses and the blocks land where intended (top-level contents: read, docs job contents: write, no other job overriding). actionlint could not be run locally -- its installer is a curl-to-shell script -- so it will be exercised by the preflight job on this PR.
`PR Build [expressions]` (Spark 4.0 / 4.1 / 4.2) failed on `expressions/misc/variant.sql`: the new full-data-schema serializability check in `CometNativeScan.isSupported` ran before the projected-schema type check, so a `struct<v: variant>` column was reported as "Native scan does not support data type struct<v:variant>" instead of the expected "type VariantType". Run the schema-support check first so the more specific type reason wins; the geospatial case that motivated the check still falls back (and no longer crashes planning), now covered by a new `expressions/misc/geospatial_types.sql` fixture. `Spark SQL Tests (Spark 4.2)` sql_core failures: - Spark 4.2 allows `FILTER (WHERE ...)` on window aggregates. Comet dropped the filter (it is only serialized for Partial mode aggregates, and window aggregates are Complete mode) and returned wrong results for `window.sql`. Decline these window expressions in `CometWindowExec`, and turn the silent drop in `aggExprToProto` into a fallback for any non-Partial mode carrying a filter. Covered by `expressions/window/window_filter.sql`. - `dev/diffs/4.2.0.diff` was reseeded from `4.1.2.diff` and missed the updates main has since made to the 4.1 diff, which caused the `SubquerySuite`, `CachedBatchSerializerNoUnwrapSuite` and `DataFrameAggregateSuite` (x4 suites) failures. Ported: the `CometHashAggregateExec` arm for SPARK-22223, the `WholeStageCodegenExec(CometColumnarToRowExec(...))` scan shapes for SPARK-26893, the codegen-aware cached-plan transition check, the `VariantEndToEndSuite` patch, `spark.comet.shuffle.enabled` for the renamed config, and `comet.version` 1.1.0-SNAPSHOT. - `tuplesketch.sql` hits the same collated-string sketch problem as `thetasketch.sql`, so it joins the ignore list. `join-nearest-by.sql` records Spark operator names in EXPLAIN goldens and `in-order-by.sql` orders by a column with ties, so both run with Comet disabled (as explain*.sql and in-limit.sql already do). Also merges apache/main, which the branch needed for the diff sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Part of #4142.
Supersedes #4208, which was opened against
4.2.0-preview4and went stale.Rationale for this change
Spark 4.2.0 is now available in Maven Central, so the
spark-4.2profile no longer has to target a preview. This is stage 3 of the bring-up described inadding_a_new_spark_version.md: turn on Spark's own SQL tests for 4.2 and move the profile onto the released version.Spark 4.2.0 is not simply
4.2.0-preview4with the qualifier dropped. It restructures the test harness, changesSpecializedGetters, repackages a dependency, and addsFILTER (WHERE ...)support for window aggregates, so several real reconciliations were needed.What changes are included in this PR?
Build
spark.versionmoves from4.2.0-preview4to4.2.0.org.apache.datasketches.memory.internal.ResourceImplinsidespark-catalyst, colliding with the transitivedatasketches-memoryjar and failing theBanDuplicateClassesenforcer rule. That class is now ignored.jetty-servlet, which Jetty 12 replaced withjetty-ee10-servlet. The stale comment claiming the pin matched preview4's Jetty version is corrected.Shims
GeographyVal/GeometryValand replacesgetGeography/getGeometryonSpecializedGetterswith a singlegetBinaryViewreturningBinaryView.CometInternalRowShimcan no longer be shared from thespark-4.1+source root, because the major, minor-plus and minor shim directories are all compile source roots (aspark-4.2copy would be a duplicate class, not an override). It is split into per-versionspark-4.1andspark-4.2copies.Comet fixes exposed by the 4.2 test runs
CometNativeScanrejects a scan whose data or partition schema contains a type with no proto representation (GEOMETRY / GEOGRAPHY on 4.2), which otherwise threwNoSuchElementExceptionduring planning instead of falling back. The native scan serializes the full data schema, not just the projected columns, so the check cannot be limited to the required schema.CometScanRulenow runs the projected/partition schema type check before this one so that a column whose type is unsupported is still reported as such (Unsupported s of type VariantType) rather than as the coarser serialization failure. New fixtureexpressions/misc/geospatial_types.sqlcovers the GEOMETRY / GEOGRAPHY fallback, including the case where the geospatial column is not projected.FILTER (WHERE ...)on a window aggregate. Comet dropped the filter silently --aggExprToProtoonly serialized it forPartialmode aggregates and window aggregates areCompletemode -- and DataFusion window expressions have no filter support anyway, sowindow.sqlgot wrong results.CometWindowExecnow declines window expressions whose aggregate carries a filter, andaggExprToProtofalls back instead of silently dropping a filter for any non-Partialmode. New fixtureexpressions/window/window_filter.sqlfails (wrong results) without the fix.Spark SQL test diff
dev/diffs/4.2.0.diffreplacesdev/diffs/4.2.0-preview4.diff, seeded from the current4.1.3.diffagainst thev4.2.0tag.SQLTestUtilsshrinks to a deprecated empty alias, andwithSQLConf,stripSparkFilterand thetest()override move intoQueryTest, withSharedSparkSessionnow extendingQueryTestdirectly. Comet's hooks (isCometEnabled, theIgnoreCometskip, and thestripSparkFilterComet cases) move toQueryTestaccordingly.isCometEnabledmust qualifyclassic.SparkSession, since a bareSparkSessionin that package resolves to the unified class.KeyGroupedPartitioningSuiteneeds no Comet change any more: 4.2 already declarescollectAllShuffles/collectShufflesasprotectedreturningSeq[ShuffleExchangeLike].HiveUDFDynamicLoadSuitedrops Comet's commented-outassume: 4.2 addsTestHiveUdfsJar, which buildshive-test-udfs.jarfrom Java sources at runtime, so the stripped-jar workaround is obsolete.tuplesketch.sqljoinsthetasketch.sqlinignoreList:tuple_sketch_estimate_double(tuple_sketch_agg_double(...))over a collated string column hits the same sketch-binary-as-UTF-8 read failure.join-nearest-by.sqlruns with--SET spark.comet.enabled = false: its EXPLAIN queries record Spark's operator names in the golden file (same treatment asexplain.sql/explain-aqe.sql/explain-cbo.sql).subquery/in-subquery/in-order-by.sqlruns with--SET spark.comet.enabled = false: one query sorts on a column with tied keys, so the order of the tied rows is implementation defined and Comet's sort does not reproduce the order recorded in the golden file (same treatment asin-limit.sql).CI
spark_4_2job to theci.ymlumbrella callingspark_sql_test_reusable.yml(Spark 4.2.0, JDK 17), plus the matchingspark_4_2filter incompute-changes.pyand output on thechangesjob. It is gated behind arun-spark-4.2-testslabel so it runs on pushes tomainbut stays off the default PR path while 4.2 is experimental.Plan stability
dev/regenerate-golden-files.sh --spark-version 4.2. Theq2,q5,q54(v1_4) andq5a(v2_7) goldens are pruned because their plans now match the shared fallback on the released 4.2.0, so the preview4-era divergences are gone.q77a(v2_7) is added and records a degraded plan: Spark 4.2 plans aOneRowRelationinto itsUnionbranches, which Comet cannot convert, so theUnions fall back and the aggregates above lose their Comet partial producer. Tracked in Spark 4.2:OneRowRelationin Union branches forces Union and downstream aggregates off Comet (TPC-DS q77a) #4949 and to be removed when that is fixed.Docs
spark_sql_test.ymlmatrix, which has since been replaced by the per-version umbrella jobs; it now documents theci.yml+compute-changes.pywiring.How are these changes tested?
./mvnw -Pspark-4.2 installagainst the released 4.2.0 builds clean, as does-Pspark-4.1, confirming the shim split does not regress 4.1.-Pspark-3.4 -Pscala-2.12also compiles.dev/diffs/4.2.0.diffwas verified to apply with no rejects to a pristinev4.2.0checkout, and patches the same files as4.1.3.diffapart from the expectedSQLTestUtilstoQueryTestswap, the 4.2-only suites, and the two fixture files above.CometSqlFileTestSuitepasses under-Pspark-4.2,-Pspark-4.1,-Pspark-4.0and-Pspark-3.5, covering the two new fixtures (skipped below 4.2 viaMinSparkVersion) and the changed scan fallback reasons.CometTPCDSV1_4_PlanStabilitySuiteandCometTPCDSV2_7_PlanStabilitySuitepass under-Pspark-4.2(97 and 32 tests, no failures).spark_4_2job exercises the diff in CI; therun-spark-4.2-testslabel is applied to this PR.