sdf-jkl commented on PR #9599: URL: https://github.com/apache/arrow-rs/pull/9599#issuecomment-4121666112
> First: That example sounds wrong? Shouldn't a variant_get that hits Variant::Null always return NULL? Variant::Null coerces to a NULL of any type, no? (unlike, say encountering Variant::Float when List<Int64> was requested). Not in strict casting. In strict casting any cast that returns `None` for non-`Null` target types errors out. Maybe we could add `Variant::Null` to other types cast support #9563, since it should be just `None`. OR add a special case for input `Null`s. > Second: But anyway, this specific code is unpacking list elements not the list itself? What specific variant value and json path did you hit problems for? That list would error on trying to cast `Null` list element to `Int64`. > Third: This code that unpacks list elements used to just append_value(value) (without ever knowing value = Variant::Null), which would produce a non-NULL value column entry containing Variant::Null. The new code specifically checks for Variant::Null and calls append_null(). But the element builder was initialized in ArrayElement mode, so append_null does the exact same thing -- produces a non-NULL value column entry containing Variant::Null. Based on that, I'm struggling to see how this change would alter behavior at all? It does end up appending the same `Variant::Null`, but it skips casting in `append_value`. That casting to a non-`Null` type from `Null` is what causing the error. ----- > Oh wait... this is going from shredded variant to some kind of ListArray, in response to variant_get(..., List<Int64>). That operation shouldn't produce shredded variant as output, so we shouldn't need to constrict it with a NullValue in the first place?? It should produce an actual list. >If we want to extract a shredded list as output, we'd need to pass variant_get(..., List<Struct>) where the list is annotated as variant extension type and the struct has value and typed_value fields of the appropriate types? Earlier I mentioned that: > `VariantToListArrowRowBuilder` builds the Arrow typed array in an interesting way. It internally stores a `StructArray` with both `value` and `typed_value` fields, so both are kept. I think that currently its sole purpose is to be used by `variant_get`, but it's not useful to convert a `VariantArray` to Arrow. > If we want to extract a shredded list as output, we'd need to pass variant_get(..., List<Struct>) where the list is annotated as variant extension type and the struct has value and typed_value fields of the appropriate types? I think you're right and it should be a separate behavior of that `variant_to_arrow` kernel. -- 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]
