scovich commented on PR #8481:
URL: https://github.com/apache/arrow-rs/pull/8481#issuecomment-3353618853
> It seems to me like the `unshred_variant` in this PR subsumes the code in
`cast_to_variant` -- we could probably switch `cast_to_variant` to create a
"dummy" variant array and call into `unshred_variant` 🤔
There's definitely some overlap and I may have missed some opportunities for
builder code reuse, but the two functions are not doing the same thing:
* `cast_to_variant` exists to convert fully strongly-typed data to binary
variant
* Among other things, this entails building a new `metadata` column from
scratch, because no metadata dictionaries exist yet.
* NOTE: The function can't even detect if a caller passes variant data --
shredded or otherwise -- because it takes a `&dyn Array` that carries no
extension type info. So it has to assume that any `StructArray` it sees is an
actual struct array whose rows should be converted to variant objects, and
whose fields might just happen to be called `metadata` and `value`.
* `unshred_variant` does what it says -- unshred already-converted variant
data
* It always works with a read-only metadata builder
* It needs to handle mixed shredding situations (including partially
shredded objects), that can require converting strongly-typed values to variant
on some rows while copying variant bytes for other rows.
* This function is less vulnerable to type mismatches because it takes
`&VariantArray` instead of `&dyn Array` -- and the `VariantArray` constructor
does at least some basic structural validation. We still technically can't
distinguish between a column with variant structure vs. an actual variant tho
-- the caller has to check the array's owning field for extension type metadata.
--
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]