Create Specific builder for typed builder trait and easier to use for lists #6863
Conversation
There was a problem hiding this comment.
I'm not sure about this, a lot of this functionality already exists
I wonder if you've given thought to a TypedListArray abstraction much like we have for DictionaryArray, ultimately the issue boils down to it being fiddly to iterate a ListArray, it isn't really a limitation of the builders
| @@ -352,6 +352,18 @@ impl ArrayAccessor for &BooleanArray { | |||
| } | |||
| } | |||
|
|
|||
| impl ArrayAccessor for BooleanArray { | |||
There was a problem hiding this comment.
This shouldn't be necessary as this is already implemented for &BooleanArray
| @@ -219,6 +219,49 @@ impl ArrayBuilder for BooleanBuilder { | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| impl SpecificArrayBuilder for BooleanBuilder { | |||
There was a problem hiding this comment.
The functionality of this trait seems to overlap with the existing extend functionality, I'm not sure what it is adding
There was a problem hiding this comment.
nothing, the only thing it add is for the list builder to use those functions as it expect a SpecificArrayBuilder trait
| if output.is_null(i) { | ||
| self.append_null(); | ||
| } else { | ||
| let current_value = output.value(i); |
There was a problem hiding this comment.
The performance of this will be very suboptimal, it would be better to iterate the offsets directly
There was a problem hiding this comment.
thanks, I did not try to make it fast now, just to make it work as a proof of concept
I think typed list array would be a great solution, my codebase knowledge is very limited |
|
@tustvold I was unable to add Also my solution does not work when creating the builder using |
|
ListBuilder already implements Extend, and adding FromIterator should be a trivial extension. The issue from your example concerns being able to extract an iterator of values from a list array, this will require adding a TypedListArray abstraction that can downcast the type-erased ListArray. I can try to find some time to write this up more properly over the next week or so |
Which issue does this PR close?
This doesn't close #6846 but it help get there
Rationale for this change
It's really hard to build nested lists, hopefully this makes it easier
What changes are included in this PR?
Created
SpecificArrayBuilderand implement it for some of the builders, I was not able to implement it for all the buildersI implement for the following:
boolean_builderfixed_size_binary_builderfixed_size_list_buildergeneric_bytes_buildergeneric_bytes_view_buildergeneric_list_buildernull_builderprimitive_builderAre there any user-facing changes?
Yes, before documentation I would like to get your feedback
Possible breaking changes as now there are 2 functions with the same name so users will be prompted to select the trait to use (I think)
TODOs