test: cover struct data columns in native shuffle - #5564
Merged
Conversation
sunchao
approved these changes
Aug 31, 2026
`CometShuffleExchangeExec.supportedSerializableDataType` allows struct data columns in native shuffle, but `CometNativeShuffleSuite` never exercised one: the only nested coverage was `array<array<int>>` and `map<int, null>`, and the word "struct" did not appear in the suite at all. The columnar suite has extensive nested coverage, so the gap was specific to native shuffle. Struct columns can only reach native shuffle as DATA columns, since `supportedHashPartitioningDataType` rejects nested types as a hash key, so these tests partition on a primitive and carry the struct along. Adds four tests: - a plain `struct<int, string>` data column - a struct data column with a null struct and a null field inside a struct - `struct<array<int>, string>` and `array<struct<int, string>>`, covering the recursive branches of the type gate - a struct data column with a map field Co-authored-by: Claude Code <noreply@anthropic.com>
viirya
force-pushed
the
test-native-shuffle-struct-coverage
branch
from
August 31, 2026 04:50
cc7148d to
59670b1
Compare
Member
Author
|
Thanks @sunchao |
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
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?
Closes #5563.
Rationale for this change
CometShuffleExchangeExec.supportedSerializableDataTypeallows struct datacolumns in native shuffle, but
CometNativeShuffleSuitenever exercised one --the only nested coverage was
array<array<int>>andmap<int, null>, and theword
structdid not appear in the suite at all.CometColumnarShuffleSuitehas extensive nested coverage, so the gap was specific to native shuffle.
Nested types in shuffle have been a real source of bugs (e.g. cd4d0e2, #5137,
on nested field nullability in
ShuffleScanExec), so a behavior the type gateexplicitly claims to support deserves an end-to-end test rather than relying on
it not regressing.
What changes are included in this PR?
Test-only. Four tests added to
CometNativeShuffleSuite:native shuffle on struct data column-- a plainstruct<int, string>native shuffle on struct data column including nulls-- a null struct and anull field inside a struct
native shuffle on deeply nested data columns--struct<array<int>, string>and
array<struct<int, string>>, covering the recursive branches of the gatenative shuffle on struct data column with map fieldAll of them partition on a primitive column and carry the struct as a data
column, because
supportedHashPartitioningDataTyperejects nested types as ahash key -- a struct can only reach native shuffle as data. The first test
carries a comment explaining this, so the arrangement is not mistaken for an
oversight.
No product code is changed.
How are these changes tested?
This PR is the tests.
CometNativeShuffleSuitepasses locally: 40 tests,40 succeeded, 0 failed (36 pre-existing + 4 new).
I also mutation-tested them to confirm they are not vacuous: temporarily
changing the native gate's
case StructType(...)tofalsemakes exactly these4 tests fail while the other 36 keep passing, so they really do assert that
struct data columns go through native shuffle, rather than passing because the
plan silently fell back to Spark.