Blizzara commented on code in PR #15854:
URL: https://github.com/apache/datafusion/pull/15854#discussion_r2068664824
##########
datafusion/substrait/src/logical_plan/producer.rs:
##########
@@ -1590,6 +1590,21 @@ pub fn from_cast(
schema: &DFSchemaRef,
) -> Result<Expression> {
let Cast { expr, data_type } = cast;
+ // since substrait Null must be typed, so if we see a cast(null, dt), we
make it a typed null
+ if let Expr::Literal(lit) = expr.as_ref() {
+ if lit.is_null() {
Review Comment:
@vbarua re
> But for compound types I'm not sure if it does the right thing for the
fold you're introducing:
> because I don't think that it make sense to flatten empty compound types.
If I understand how this would work, something like
There is a comment in the code of "is_null" before those:
> // arr.len() should be 1 for a list scalar, but we don't seem
to
> // enforce that anywhere, so we still check against array
length.
I haven't validated if that is correct, but I guess the idea is that a
SV:List is null if the _outer, "one-row" list itself_ is null. If that list is
not null, then the inner list can be empty. So an empty compound type would
have first the `arr` with `arr.len() = 1, and `arr.null_count() = 0`, passing
the check; then arr.values(0) would be another list which can have its own
length and null count (where e.g. both would then be zero for an empty list).
So unless I misunderstood what you meant, or there's some bug that I'm not
aware of / or a difference in the behavior to what I think it is, I do believe
the earlier code was also correct. However the change to only deal with SV:Null
seems good to me as well.
(I do find it annoying how complicated the compound types for SV are to
understand. Why is SV::List a single-row ListArray instead of just a
Vec<ScalarValue> ? But maybe there's a reason, and in any case changing that is
probably too much of a hassle now, and totally outside the scope of this PR 😄 )
--
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]
