Skip to content

Native shuffle has no test coverage for struct data columns #5563

Description

@viirya

Is your feature request related to a problem or challenge?

CometShuffleExchangeExec.supportedSerializableDataType (the native shuffle
data-column type gate) allows struct data columns, recursing through
StructType / ArrayType / MapType:

case StructType(fields) =>
  fields.nonEmpty && fields.forall(f => supportedSerializableDataType(f.dataType))
case ArrayType(elementType, _) =>
  supportedSerializableDataType(elementType)
case MapType(keyType, valueType, _) =>
  supportedSerializableDataType(keyType) && supportedSerializableDataType(valueType)

But CometNativeShuffleSuite never exercised a struct data column. The only
nested coverage in that suite is:

  • native shuffle on nested array -- array<array<int>>
  • native shuffle with Map[_, NullType] column -- map<int, null>

The word struct does not appear anywhere in the suite. By contrast,
CometColumnarShuffleSuite has extensive nested coverage (struct with nulls,
array/struct as map key/value, array<map>, many map value types), so the gap
is specific to native shuffle.

This matters because nested types in shuffle have been a real source of bugs --
e.g. cd4d0e2 ("fix: normalize nested field nullability in ShuffleScanExec and
ExpandExec", #5137), where Arrow treats nested field nullability as part of
DataType identity and the decoded shuffle block had to be reconciled against
the catalyst-declared schema. A behavior the gate explicitly claims to support,
with no end-to-end test, can regress silently.

Note that struct columns can only reach native shuffle as data columns:
supportedHashPartitioningDataType rejects nested types as a hash key, so a
query partitioning on a struct falls back to Spark. Tests therefore need to
partition on a primitive and carry the struct along.

Describe the solution you'd like

Add native shuffle tests covering struct data columns, including nulls, the
recursive nesting branches of the type gate (struct<array<...>>,
array<struct<...>>), and a struct containing a map field.

Describe alternatives you've considered

None -- this is purely additional test coverage for existing, already-supported
behavior. No product code change is proposed.

Additional context

Verified by mutation testing: temporarily changing the native gate's
case StructType(...) to false makes exactly the new tests fail and leaves
the other 36 tests in the suite passing, confirming they assert native struct
shuffle rather than passing vacuously via a silent fallback to Spark.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions