test: restore Spark 4.1 Variant shredding suites - #5745
Conversation
sunchao
left a comment
There was a problem hiding this comment.
Correctness
Reviewed dcc2d4f8 against authoritative base 0b4549fc. The Spark 4.1.3 patch still disabled both Variant shredding suites for the now-closed #2209. This restores their existing assertions and retains one test-level exclusion for the separate annotation-validation issue #5741. The authored change affects only test selection in dev/diffs/4.1.3.diff. It does not implement native Variant support or change scan behavior.
The existing SQLTestUtils.test hook ignores an IgnoreComet tag only when Comet is enabled. Removing the two IgnoreCometSuite mixins therefore restores ordinary registration, while the named annotation test remains ignored in Comet runs. With Comet disabled, that hook delegates to normal registration and preserves all 15 cases. This disabled-mode conclusion is source-derived, not a new execution result.
For semantic context, the maintained Spark 4.0 branch at 03f28fc4 marks shredded Variant projections with per-field VariantMetadata. The unchanged Comet 4.x shim delegates that detection to Spark, and CometScanTypeChecker rejects the marked struct to preserve fallback. The maintained tests exercise null versus missing values, nested arrays/objects, malformed input, cast errors, numeric boundaries and session time zones. Restoring their coverage does not weaken those assertions or alter ANSI/error handling. The maintained Spark 3.5/4.0 integration paths are unchanged. Maintained 3.4/4.1 source branches are unavailable locally, so I do not claim maintained-source coverage for those versions or substitute another Spark checkout.
Validation and CI
Local checks validated both unified patches and confirmed that the other 95 shared patch sections are unchanged after excluding index headers. The POM change is solely its result hash. The authored diff passes git diff --check.
At 2026-09-08 02:00 UTC, checks report 13 successes, 12 skipped and one neutral, with no failures. All seven Spark 4.1.3/JDK 17 test groups and the native/JVM build passed. I verified merge 119d137a, whose parents are exactly the assigned base and head. Its difference from the base equals the authored change. Build logs show application of this patch, and the compiled Spark artifact digest matches the one downloaded by the test jobs.
The Apache CI core-1 log records all eight VariantShreddingSuite tests and six ParquetVariantShreddingSuite tests passing, with only the named #5741 case ignored. These are Comet-enabled Spark test results, including existing fallback behavior. They do not prove native Variant execution or that #5741 is fixed. No local Spark/JNI rebuild or execution was performed. No P1/P2 correctness finding.
Performance
This adds 14 existing tests back to CI and introduces no runtime allocation, copying or scan overhead. The test-registration hook is unchanged, and the new tag is used only during registration. A query microbenchmark would not validate this test-selection change. The passing suite timings are test-run evidence, not a Comet-versus-Spark performance comparison.
Design
Replacing broad suite exclusions with one named, issue-linked exclusion restores useful regression coverage while keeping the known error-policy mismatch visible. It also follows the existing mechanism used elsewhere in this patch. The remaining exclusion is scoped to the test containing the failing annotation check, and all its assertions remain available when Comet is disabled. No further design change is needed for this PR.
Abstraction & complexity
The change removes two mixins and reuses the existing IgnoreComet tag and registration hook. It adds no helper hierarchy, custom runner or special configuration path. The issue link makes the remaining exclusion straightforward to revisit. No actionable complexity concern or other verified P1/P2 finding. Approving this revision.
Which issue does this PR close?
Part of #5569 (Variant shredding suites).
Rationale for this change
The Spark 4.1.3 patch still excludes both Variant shredding suites for #2209, which was fixed by #4084. Restore their coverage while keeping a focused exclusion for #5741.
What changes are included in this PR?
Remove
IgnoreCometSuitefromVariantShreddingSuiteandParquetVariantShreddingSuite. Tag onlyvariant logical type annotation - ignore variant annotationwithIgnoreComet, linked to #5741. This restores 14 tests with Comet enabled and preserves all 15 with Comet disabled.Regenerated
dev/diffs/4.1.3.difffrom Spark v4.1.3 sources. Regeneration also refreshes the existing pom.xml result hash without changing its patch content.How are these changes tested?
Spark 4.1.3 / JDK 17 fork CI passed, including all seven Spark SQL test groups. The restored suites ran 14 tests successfully, with only the #5741 case ignored. The Hive job was rerun after a runner shutdown.
The regenerated patch applies cleanly to Spark v4.1.3, and the modified Spark sources pass
git diff --check.