Uh oh!
There was an error while loading. Please reload this page.
Add ColumnarValue::values_to_arrays, deprecate columnar_values_to_array - #9114
Conversation
| let args = columnar_values_to_array(args)?; | ||
| // Expand the arguments to arrays (this is simple, but inefficient for | ||
| // single constant values). | ||
| let args = ColumnarValue::values_to_arrays(args)?; |
There was a problem hiding this comment.
This is the basic change -- make the logic a function of ColumnarValue
| /// # Errors | ||
| /// | ||
| /// If there are multiple array arguments that have different lengths | ||
| pub fn values_to_arrays(args: &[ColumnarValue]) -> Result<Vec<ArrayRef>> { |
There was a problem hiding this comment.
This is a different algorithm than columnar_values_to_array as it also handles mixed ScalarValue and ArrayRefs
| use super::*; | ||
| #[test] | ||
| fn values_to_arrays() { |
Omega359
commented
Feb 2, 2024
In the make_date code I was attempting to not actually expand the scalar into a full array but rather use in place. It was one of the reasons why I was asking about a SingleValueArray as with something like that we could have the best of both - easy processing and no data duplication. I feel though that would likely be a lot more work than it seems it would be otherwise |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com>
alamb
commented
Feb 5, 2024
Thanks again for the review @viirya |
Which issue does this PR close?
Part of #8045
Rationale for this change
While working on implementing
ArrayToStringin #9113 I found that many of the array functions support arbitrary combinations of arguments that areColumnarValuetoArrayby converting all the arguments to arrays and then calling a function that works on arrays.I think this is a common desire when implementing functions, but it is not always clear how to do this.
For example I think @Omega359 had to implement something similar in
to_datein #9040@viirya added a helper in #8962 which improves the situation, but I think this function may be hard to find (it is in
datafusion_physical_expr) and it doesn't handle the case where the arguments are a mix ofColumnarValue::ScalarandColumnarValue::ArrayWhat changes are included in this PR?
ColumnarValue::values_to_arrayscolumnar_values_to_arrayAre these changes tested?
Yes, new tests
Are there any user-facing changes?
New function, better docs and tests