You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "rspears74 (via GitHub)" <gi...@apache.org> on 2023/12/08 20:45:50 UTC
[I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
rspears74 opened a new issue, #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472
### Describe the bug
I am trying to use `ScalarValue::List` to store `f64` values for the `state` in a UDAF. It was working well in datafusion version 32, but I upgraded to datafusion 33, only to find that the API had changed for `ScalarValue::List` to accept an `ArrayRef` rather than an `Option<Vec<ScalarValue>>`. I thought I had converted my code correctly, but I'm getting the following error:
```
Error: External(Internal("Inconsistent types in ScalarValue::iter_to_array. Expected Float64, got List([PrimitiveArray<Float64>\n[\n 43.0,\n 1.0,\n 43.0,\n 1.0,\n 43.0,\n 1.0,\n 43.0,\n 1.0,\n 43.0,\n 3.0,\n ...180 elements...,\n 62.0,\n 3.0,\n 62.0,\n 2.0,\n 63.0,\n 1.0,\n 63.0,\n 1.0,\n 63.0,\n 1.0,\n]])"))
```
First of all, I can't tell exactly where this error is coming from (and RUST_BACKTRACE=1 doesn't do anything), even after adding some log statements at the beginning and end of each `Accumulator` function (what I mean by this is that this error doesn't seem to be occurring "in the middle" of any of the `Accumulator` functions I've implemented).
I am serializing my list via `ScalarValue::List(Arc::new(Float64Array::from(floats)))` where `floats: Vec<f64>`
And deserializing my list via something like this:
```
if let ScalarValue::List(float_vals) = &v[0] {
let floats: Vec<f64> = float_vals
.as_any()
.downcast_ref::<Float64Array>()
.expect("Failed to downcast")
.values()
.collect()
}
```
Finally, my `state_type` in the `create_udaf` function is:
```
Arc::new(vec![
DataType::List(Field::new(
"item",
DataType::Float64,
true
).into())
```
Via logging, I seem to get successful calls to `state` and `update_batch`.
I'm not sure if I'm making a mistake somewhere, or if this is a bug. But it seems like somewhere in the guts of the `Accumulator`, something is not working correctly. Happy to help fix the bug if one can be identified.
### To Reproduce
Define a UDAF `Accumulator` that uses `ScalarValue::List` to serialize its state, and use the aggregate function.
### Expected behavior
Successful aggregation of the values into a new dataframe column.
### Additional context
_No response_
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1851878232
Agree, I had thought about it but did not come out a nice solution. The problem is that ScalarValue is enum not a function created with new(), so we can only let it panic somewhere...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "rspears74 (via GitHub)" <gi...@apache.org>.
rspears74 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1851159015
The docs say that it accepts an `Arc<dyn Array>`, and it compiles as I have it written
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "rspears74 (via GitHub)" <gi...@apache.org>.
rspears74 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1852317140
Understood. Can the signature of `ScalarValue::List` not just be changed from `ArrrayRef` to `Arc<ListArray>` (similar for `FixedSizeList` and `LargeList`)? That at least solves half the problem. I'm not trying to tell anyone how to maintain their project, but if this was my project, I would not be okay leaving `ScalarValue::List` like this. More people are going to run into issues around this in the future.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1851156095
> ScalarValue::List(Arc::new(Float64Array::from(floats)))
Note that ScalarValue::List only accept ListArray. In your example, it should be something like ListArray(Float64Array::from(floats)) where ListArray is length = 1.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "rspears74 (via GitHub)" <gi...@apache.org>.
rspears74 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1851188247
Alright, so after constructing a `ListArray` with my `Float64Array` as its `values`, it works! But it's certainly not clear from the docs that you have to pass in a `ListArray` of length 1. I would think there should be some way to enforce that outside of failing in a random spot at runtime.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #8472: `ScalarValue::List` not working as expected in UDAF state
URL: https://github.com/apache/arrow-datafusion/issues/8472
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1852722752
> Understood. Can the signature of `ScalarValue::List` not just be changed from `ArrayRef` to `Arc<ListArray>` (similar for `FixedSizeList` and `LargeList`)? That at least solves half the problem. I'm not trying to tell anyone how to maintain their project, but if this was my project, I would not be okay leaving `ScalarValue::List` like this. More people are going to run into issues around this in the future.
I think this would be a very interesting idea to pursue. Thank you for the suggestion @rspears74
Are you (or @jayzhan211 ) interested in doing so? If not, that is fine, and I will file another ticket with a more approachable description. However, if one of you who already has this context is going to do so I won't bother writing it up more carefully :)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1850217454
Maybe @jayzhan211 or @Weijun-H have some thoughts
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] `ScalarValue::List` not working as expected in UDAF state [arrow-datafusion]
Posted by "rspears74 (via GitHub)" <gi...@apache.org>.
rspears74 commented on issue #8472:
URL: https://github.com/apache/arrow-datafusion/issues/8472#issuecomment-1852734782
I'll have to familiarize myself with the contribution guide, but I'd certainly like to take a whack at this @alamb
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org