-
Notifications
You must be signed in to change notification settings - Fork 1k
Variant to arrow utf8 #8600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Variant to arrow utf8 #8600
Conversation
|
||
perfectly_shredded_to_arrow_primitive_test!( | ||
get_variant_perfectly_shredded_utf8_as_utf8, | ||
DataType::Utf8, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add tests for other types(LargeUtf8/Utf8View
) here?
The test here wants to cover the variant_get
logic, and the tests added in variant_to_arrow.rs
were to cover the logic of the builder?
TimestampNano(VariantToTimestampArrowRowBuilder<'a, datatypes::TimestampNanosecondType>), | ||
TimestampNanoNtz(VariantToTimestampNtzArrowRowBuilder<'a, datatypes::TimestampNanosecondType>), | ||
Date(VariantToPrimitiveArrowRowBuilder<'a, datatypes::Date32Type>), | ||
StringView(VariantToUtf8ViewArrowBuilder<'a>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We added StringView
to the PrimitiveVariantToArrowRowBuilder
and the other two to StringVariantToArrowRowBuilder
, is there a particular reason for this?
|
||
define_variant_to_primitive_builder!( | ||
struct VariantToUtf8ArrowRowBuilder<'a> | ||
|item_capacity, data_capacity: usize| -> StringBuilder {StringBuilder::with_capacity(item_capacity, data_capacity)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|item_capacity, data_capacity: usize| -> StringBuilder {StringBuilder::with_capacity(item_capacity, data_capacity)}, | |
|item_capacity, data_capacity: usize| -> StringBuilder { StringBuilder::with_capacity(item_capacity, data_capacity) }, |
Which issue does this PR close?
Rationale for this change
Add support for Variant::Utf-8, LargeUtf8, Utf8View. This needs to add a new builder VariantToStringArrowRowBuilder, because LargeUtf8, Utf8View are not ArrowPritimitiveType's
What changes are included in this PR?
data_capacity
tomake_string_variant_to_arrow_row_builder
to support string types.make_string_variant_to_arrow_row_builder
invariant_get
to include the variable.Are these changes tested?
Added a variant_get test for utf8 type and created two separate tests for largeUtf8 and Utf8view because these types can't be shredded.
Are there any user-facing changes?
No