arrow-select: Implement concat for RunArrays
#7487
Conversation
It's unclear whether the added work and complexity of merging the last run between consecutive arrays with the first one of the next array being concatenanted is worth the potential benefit. For now this implementation focuses on the ability to concatenate `RunArray`s in the first place.
There was a problem hiding this comment.
Thank you @brancz -- I think this looks quite good to me. With a few more tests I think it will be ready to merge.
It's unclear whether the added work and complexity of merging the last run between consecutive arrays with the first one of the next array being concatenanted is worth the potential benefit.
I agree -- and the approach in this PR has the nice benefit that it is very simply to explain
|
|
||
| // Concatenate the arrays - this should now work properly | ||
| let result = concat(&[&array1, &array2]).unwrap(); | ||
| let result_run_array = result |
There was a problem hiding this comment.
perhaps you could use the new as_run() methods added elsewhere in this PR
| let values1 = Int32Array::from(vec![10, 20]); | ||
| let array1 = RunArray::try_new(&run_ends1, &values1).unwrap(); | ||
|
|
||
| let run_ends2 = Int32Array::from(vec![2, 4]); |
There was a problem hiding this comment.
Can you please change the run ends for the second array so they aren't the same as the first? Also it would be good to include a run with length 1
Something like this perhaps:
| let run_ends2 = Int32Array::from(vec![2, 4]); | |
| let run_ends2 = Int32Array::from(vec![1, 4]); |
| } | ||
|
|
||
| #[test] | ||
| fn test_concat_run_array_with_nulls() { |
There was a problem hiding this comment.
Can you also please add a test for:
- calling
concatwith 1 RunArray - Calling
concatwith 3 Run arrays
It's unclear whether the added work and complexity of merging the last run between consecutive arrays with the first one of the next array being concatenanted is worth the potential benefit. For now this implementation focuses on the ability to concatenate
RunArrays in the first place.Part of apache/datafusion#16011
What changes are included in this PR?
Support for concatenating
RunArrays.Are there any user-facing changes?
No, this is strictly additive and doesn't even add any new APIs, just adds support for
RunArrays in an existing API.@alamb