[AutoSparkUT] Fix map_zip_with lambda result nullability [fast-ut] [databricks] - #15900
Conversation
Signed-off-by: Allen Xu <allxu@nvidia.com>
Greptile SummaryThe PR corrects
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or test-quality issues identified. The production change corrects output schema metadata without adding runtime work, and the updated regression directly compares CPU and GPU results while requiring GPU project execution. Important Files Changed
Reviews (1): Last reviewed commit: "Fix map_zip_with result nullability" | Re-trigger Greptile |
There was a problem hiding this comment.
🟢 Approval recommended
The change aligns GPU output schema nullability with Spark’s lambda nullability semantics and the updated regression test now validates the corrected behavior directly.
Pull request overview
This PR fixes a correctness bug in the GPU implementation of map_zip_with by making the result MapType.valueContainsNull follow the lambda’s nullability contract (matching Spark CPU behavior), and updates the integration regression to stop masking failures now that the behavior is corrected.
Changes:
- Update
GpuMapZipWith.dataTypeto setvalueContainsNull = function.nullableinstead of deriving it from the two input map schemas. - Remove the issue-scoped strict xfail + exception translation in
test_map_zip_with_decimal_identity, reverting it to a direct GPU/CPU equality assertion.
File summaries
| File | Description |
|---|---|
| sql-plugin/src/main/scala/com/nvidia/spark/rapids/higherOrderFunctions.scala | Align GpuMapZipWith output map value nullability with the lambda expression’s nullable flag to prevent incorrect non-null schema/results. |
| integration_tests/src/main/python/map_test.py | Remove the strict xfail scaffolding and restore a direct assertion for the decimal identity map_zip_with regression test. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
build |
JaCoCo production line coverage: +3 lines (
sql-plugin +3; shim 350, fix-line intersection from the seed-0 regression)Fixes #15783.
Contributes to #15896.
Description
GpuMapZipWithderived the result map'svalueContainsNullflag from the two input maps. Spark derives it fromfunction.nullable: when the input maps have different key sets, either lambda value argument can be null even if both input map value types are declared non-null. The old GPU schema could therefore declare a nullable lambda result as non-null and produce wrong results.This change makes the GPU result type follow the lambda's nullability contract and removes the strict xfail from the Decimal identity regression. The focused regressions also require
GpuProjectExec, so their successful GPU/CPU comparison proves that the repaired expression executed on GPU.Performance impact: none expected. This only changes Catalyst output-type metadata during expression planning, replacing an input-map boolean calculation with the already-computed lambda
nullableflag; it adds no row/batch runtime work, allocation, or GPU kernel.AI assistance: The change and PR description were prepared with Codex assistance.
Local validation:
3580969fb1b47c9c58a04ff4f7daa2b8f56cccef: 16/16 reactor modules succeeded.2 passed; both Decimal parameterizations were markedINJECT_OOM, andGpuProjectExecwas validated.map_test.py, Spark 3.5, seed 0 without OOM injection:710 passed, 5 skipped.sql-pluginproduction lines covered.The exact Dataproc Serverless 2.2 / Scala 2.13 rerun from #15896 is not part of this local validation.
Checklists
Documentation
Testing
(Please provide the names of the existing tests in the PR description.)
Performance