PDGGK opened a new pull request, #27035:
URL: https://github.com/apache/datafusion/pull/27035
## Which issue does this PR close?
- Closes #27012.
## Rationale for this change
`map_entries` panics when the input map's entry fields are not exactly `key`
(non-null) and `value` (nullable) with no metadata. One such case is a map
whose values are declared non-nullable. The repo's own `parquet_map.parquet`
has such a column, and on 55.2.0 it brings down `datafusion-cli`:
```
$ datafusion-cli -q -c "SELECT map_entries(ints) FROM 'parquet_map.parquet'
LIMIT 1;"
thread 'main' panicked at
.../arrow-array-59.2.0/src/array/list_array.rs:272:54:
called `Result::unwrap()` on an `Err` value: InvalidArgumentError("ListArray
expected data type Struct(\"key\": non-null Utf8, \"value\": Int64) got
Struct(\"key\": non-null Utf8, \"value\": non-null Int64) for \"item\"")
```
The same panic happens when the entry fields have other names. One example
is `keys`/`values`, the arrow-rs `MapBuilder` default:
```
> SELECT map_entries(arrow_cast(MAP {'a': 1, 'b': NULL}, 'Map("entries":
non-null Struct("keys": non-null Utf8, "values": Int64), unsorted)'));
thread 'main' panicked at .../list_array.rs:272:54:
... InvalidArgumentError("ListArray expected data type Struct(\"key\":
non-null Utf8, \"value\": Int64) got Struct(\"keys\": non-null Utf8,
\"values\": Int64) for \"item\"")
```
It also happens when the entry fields carry metadata, such as
`PARQUET:field_id`. `map_keys`, `map_values`, `map_extract`, `cardinality` and
`m['k']` all work on the same inputs.
The cause is that `map_entries` declares its result as `List(Struct(key,
value))` with a nullable `value` field, as `return_type` does, but wraps the
input's entries array unchanged. `ListArray::new` then panics because the two
struct types differ.
## What changes are included in this PR?
`map_entries_inner` rebuilds the entries `StructArray` with the declared
`key`/`value` fields. It reuses the input's child arrays, so no data is copied.
The result always matches the declared return type, and that type does not
change.
One alternative would be to carry a non-nullable value through to the return
type, as `map_values` does since #15901. That changes the result type for those
inputs, and it would still need the rebuild for other field names or metadata.
So I kept the existing declared type. Happy to switch if reviewers prefer the
other approach.
## What is the testing strategy for this PR?
New cases in `map.slt`:
- `map_entries(ints)` on the `data` table (`parquet_map.parquet`,
non-nullable values). It now returns `[{key: bytes, value: 49960}]` and `[{key:
bytes, value: 49689}]` for the top two rows.
- `map_entries` on a map cast to non-nullable values, and on a map cast to
`keys`/`values` field names.
- `arrow_typeof` of the result, to pin that the declared type is unchanged:
`List(non-null Struct("key": non-null Utf8, "value": Int64))`.
Each new query panics without the change to `map_entries.rs`, with the error
from the issue. Locally I ran `map.slt` and the `datafusion-functions-nested`
unit tests. I left the full workspace and extended test runs to CI.
## Are there any user-facing changes?
`map_entries` no longer panics on these maps and returns the entries. There
is no API change and the result type is unchanged.
--
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]
