Smallfu666 opened a new pull request, #5227: URL: https://github.com/apache/datafusion-comet/pull/5227
## Which issue does this PR close? Related to #5097. (Intentionally not `Closes` — see the scope note; the "entries null buffer" part of the issue is not representable through safe Arrow APIs.) ## Rationale for this change `cast_map_to_map` had two real bugs and one latent hazard: - It used the **source** `sorted` flag for the result, so a cast to a map type with a different `sorted` flag returned the wrong type. - It rebuilt the entries/key/value fields from scratch, **dropping field metadata** (and target nullability), so `data_type()` did not equal the requested target. - Casting the **key type** of a `sorted` map can reorder keys (e.g. sorted Utf8 `["10","2"]` → Int32 `[10,2]`), which would silently produce a `Map(.., sorted=true)` that is no longer sorted. ## What changes are included in this PR? - **Rename-only fast path**: when key/value types and sort order are unchanged (the common Parquet `key_value` → Spark `entries` relabel), delegate to arrow's `cast`, which relabels to the target fields and preserves their metadata with no value transformation. - **Recursive/hand-built path**: when a child type changes, recurse with Comet's `cast_array`; when the sort order downgrades (`true → false`), hand-build with the target flag. `try_new` is used so a malformed target returns `Err` instead of panicking, and the result `data_type()` equals the requested target. - **Sorted-order safety**: reject `to_sorted && (!from_sorted || key-type changed)` — a cast cannot manufacture or preserve an ordering across a key-type change. - **Field-count guard** before indexing entries `[0]/[1]` — a malformed entries struct (0/1/3+ fields) returns `Err`, never panics. Scope note: an entries **struct** carrying a non-`None` null buffer is not constructible through the inspected safe Arrow constructors (`StructArray::new`/`try_new` and `MapArray` normalize an all-valid buffer to `None`), so entries-level null-buffer "preservation" is not an observable property and is not claimed. Map-**level** null preservation is covered. ## How are these changes tested? 16 unit tests in `cast.rs`: rename-only fast path (full target schema + unchanged values/offsets/map nulls); sorted equal / `true→false` / `false→true`(Err) / `true→true` key-type-change(Err) / value-only-cast; metadata + child casts; empty maps; mixed null/empty rows; sliced maps; key+value casts; malformed targets (0/1/3 entry fields, non-nullable field with null) returning `Err` without panic. -- 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]
