andygrove opened a new issue, #7062:
URL: https://github.com/apache/datafusion-comet/issues/7062
### Describe the bug
A shuffle that runs on Comet's JVM columnar shuffle fails the query when one
of its columns is a struct with a `NullType` field, such as `named_struct('x',
a, 'n', null)`. The row-to-Arrow conversion panics:
```
org.apache.comet.CometNativeException: internal error: entered unreachable
code: Unsupported data type of struct field: Null
at
datafusion_comet_shuffle::spark_unsafe::row::append_struct_fields_field_major
at datafusion_comet_shuffle::spark_unsafe::row::append_columns
at
datafusion_comet_shuffle::spark_unsafe::row::process_sorted_row_partition
at comet::execution::jni_api::write_sorted_file
at
comet::execution::jni_api::Java_org_apache_comet_Native_writeSortedFileNative
```
Spark returns the rows. Both field-major struct paths in
`native/shuffle/src/spark_unsafe/row.rs` lack a `DataType::Null` arm and fall
through to `unreachable!`. `append_struct_fields_field_major` handles a
top-level struct column, and `append_nested_struct_fields_field_major` handles
a struct inside a struct:
https://github.com/apache/datafusion-comet/blob/3a4a15940983e4bb9a9a74c816ee51e2cf6d9e17/native/shuffle/src/spark_unsafe/row.rs#L1042-L1047
https://github.com/apache/datafusion-comet/blob/3a4a15940983e4bb9a9a74c816ee51e2cf6d9e17/native/shuffle/src/spark_unsafe/row.rs#L582-L587
`append_field` in the same file already has the arm. The JVM columnar
shuffle's type check accepts `NullType` at any depth, so nothing keeps these
schemas away from the converter, and `Null` is the only type that check accepts
that the two functions are missing.
https://github.com/apache/datafusion-comet/blob/3a4a15940983e4bb9a9a74c816ee51e2cf6d9e17/native/shuffle/src/spark_unsafe/row.rs#L272-L275
With the default configuration this hits every round-robin repartition of
such a column, because native round robin is off by default and those shuffles
run on JVM columnar shuffle. It also hits a hash shuffle that runs on JVM
columnar shuffle, for example under `spark.comet.shuffle.mode=jvm`. A hash
shuffle over a native child runs on native shuffle and works. Arrays and maps
that hold nulls (`array(null)`, `map(k, null)`, `array(named_struct('x', a,
'n', null))`) also work.
### Steps to reproduce
```sql
CREATE TABLE t (a INT) USING parquet;
INSERT INTO t SELECT CAST(id AS INT) FROM range(1000);
-- top-level struct column
SELECT /*+ REPARTITION(4) */ a, named_struct('x', a, 'n', null) AS st FROM t;
-- struct inside a struct
SELECT /*+ REPARTITION(4) */ a, named_struct('s', named_struct('x', a, 'n',
null)) AS st FROM t;
-- hash partitioning on JVM columnar shuffle
SET spark.comet.shuffle.mode=jvm;
SELECT a, named_struct('x', a, 'n', null) AS st FROM t DISTRIBUTE BY a;
```
All three fail with the panic above. I reproduced them on c3316fbe39 with
the Spark 4.1 profile, and the code involved is unchanged on main at 3a4a159409.
### Expected behavior
The queries should return the same rows as Spark. Every value of a
`NullType` field is null, so a `DataType::Null` arm in both functions that
appends one null per row, as `append_field` does, should be enough. A
regression test with a top-level and a nested struct would cover both paths.
### Additional context
#6977 proposes merging the two field-major functions, which would make an
arm like this one change instead of two.
--
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]
