feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns - #10831
Conversation
|
|
||
| /// Convert a dictionary-typed array with string or binary typed values | ||
| /// to one with string or binary view typed values. | ||
| fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> { |
There was a problem hiding this comment.
DictionaryBuffer only works with an OffsetBuffer, not a ViewBuffer, so this extra conversion step is required:
Maybe DictionaryBuffer could be refactored to avoid this or a new flavour of DictionaryBuffer could be created that works with view types, but that seems like it would be a much more complex change. This probably isn't too performance sensitive as it only needs to convert the dictionary values.
I'm happy to look into making this work without the extra copy if we think that's needed though.
There was a problem hiding this comment.
yea after this is shipped I think this is worth making a follow up issue for to track.
Rich-T-kid
left a comment
Rich-T-kid
left a comment
There was a problem hiding this comment.
this PR looks very good. I think we can squeeze out a bit more performance when creating the output array in convert_values_to_view
|
|
||
| /// Convert a dictionary-typed array with string or binary typed values | ||
| /// to one with string or binary view typed values. | ||
| fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> { |
There was a problem hiding this comment.
yea after this is shipped I think this is worth making a follow up issue for to track.
|
Thanks for taking a look @Rich-T-kid, I've pushed a change to remove the ArrayDataBuilder validation. |
|
will take a second look tommorow 👍 |
Jefffrey
added
the
enhancement
Rich-T-kid
left a comment
Rich-T-kid
left a comment
There was a problem hiding this comment.
LGTM. I wasn't aware that this previously wasn't supported, should be very useful for apache/datafusion#24111. thank you @adamreeve
|
thanks @adamreeve & @Rich-T-kid |
3 participants
Which issue does this PR close?
Dictionary(_, Utf8View)typed column to Parquet #10830.Rationale for this change
Allows reading and writing this data type with Parquet. This is particularly useful for working with Polars categorical types, which use a
Dictionary(UInt32, Utf8View)representation.What changes are included in this PR?
BinaryViewandUtf8Viewvalue types inByteArrayDictionaryReaderto allow reading this data back as the same type.Are these changes tested?
Yes, I've added a new unit test to cover this.
Are there any user-facing changes?
Yes, this is new user-facing functionality.