sunchao opened a new pull request, #58686:
URL: https://github.com/apache/spark/pull/58686

   Related JIRA: 
[SPARK-55411](https://issues.apache.org/jira/browse/SPARK-55411), which 
introduced the affected behavior. A separate follow-up JIRA for this specific 
bug is pending creation; this draft currently uses the originating issue key.
   
   Draft: master-branch native validation remains pending.
   
   ### What changes were proposed in this pull request?
   
   Preserve nullable bucket keys in the in-memory test catalog. A null field 
contributes zero to the fixture's additive bucket hash, and `BucketFunction` 
explicitly uses zero for a null key before reading the primitive value. This 
keeps the two test helpers consistent for nulls.
   
   Add regression coverage for nullable inserts and readback across the 
fixture's supported input types, including the resulting bucket key. Add a 
repeated-expression check that evaluates a non-null key followed by null, 
exercising the reused `SpecificInternalRow`.
   
   ### Why are the changes needed?
   
   SPARK-55411 ([#54182](https://github.com/apache/spark/pull/54182), with the 
4.0 backport in [#54260](https://github.com/apache/spark/pull/54260)) replaced 
the fixture's hash calculation with a typed match. The old calculation skipped 
null values, but the replacement omitted a null case. As a result, inserting 
null into a bucketed test table throws `IllegalArgumentException` in 
`InMemoryBaseTable.getKey`.
   
   The paired `BucketFunction` also reads `getLong(1)` without checking the 
null flag. When the expression reuses a `SpecificInternalRow`, a null key can 
read the preceding key's primitive payload. An explicit null check avoids that 
stale-value result.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. The changes are confined to test fixtures and regression tests.
   
   ### How was this patch tested?
   
   Added two tests in `KeyGroupedPartitioningSuite`:
   
   - `SPARK-55411: bucket transform preserves null keys`: inserts and reads 
null for TINYINT, SMALLINT, INT, BIGINT, TIMESTAMP, TIMESTAMP_NTZ, STRING, and 
BINARY, and checks that each null is assigned bucket zero.
   - `SPARK-55411: bucket function handles null after a non-null key`: 
evaluates the same bound expression for `5L`, null, `-5L`, and null, expecting 
`1`, `0`, `3`, and `0` with four buckets.
   
   `git diff --check` and a static changed-source line-length check passed. 
**This master-branch proposal has not been compiled or run natively.** The 
identical new test block and bucket-guard logic passed both cases in a 
selected-source branch-4.0 adaptation with cached dependencies. Removing only 
the two guards reproduced `(null, ByteType)` during the first TINYINT insertion 
and `1 did not equal 0` at the first NULL-after-5 assertion; exact identities 
and loaded fresh-class origins were checked. The failing controls do not 
establish later loop iterations. The guarded positive cases cover all listed 
types and the full expression sequence.
   
   The combined 4.0 broad run completed 244 cases with 237 passes and seven 
function-ordering failures also reproduced on its production baseline. Its nine 
changed files passed Scalastyle with zero errors or warnings. This does not 
validate master's different toolchain or execution architecture, and is not 
standalone validation, a full build or CI result. A prior NULL-control audit 
incorrectly required a reader on a pre-read failure path; that rejected attempt 
is preserved separately, and the final control run used a separately reviewed 
phase-appropriate requirement.
   
   Planned focused validation:
   
   ```text
   build/sbt "sql/testOnly *KeyGroupedPartitioningSuite -- -z SPARK-55411"
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: OpenAI Codex (version not exposed in this session).
   


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