scovich commented on code in PR #8516:
URL: https://github.com/apache/arrow-rs/pull/8516#discussion_r2399098994
##########
parquet-variant-compute/src/type_conversion.rs:
##########
@@ -60,6 +65,44 @@ impl_primitive_from_variant!(datatypes::UInt64Type, as_u64);
impl_primitive_from_variant!(datatypes::Float16Type, as_f16);
impl_primitive_from_variant!(datatypes::Float32Type, as_f32);
impl_primitive_from_variant!(datatypes::Float64Type, as_f64);
+impl_primitive_from_variant!(
+ datatypes::Date32Type,
+ as_naive_date,
+ Date32Type::from_naive_date
+);
+
+pub(crate) trait TimestampFromVariant: ArrowTimestampType {
+ fn from_variant(variant: &Variant<'_, '_>) -> Option<Self::Native>;
+}
+
+macro_rules! impl_timestamp_from_variant {
+ ($timestamp_type:ty, {
+ $(($variant_pattern:pat, $conversion:expr)),+ $(,)?
+ }) => {
+ impl TimestampFromVariant for $timestamp_type {
+ fn from_variant(variant: &Variant<'_, '_>) -> Option<Self::Native>
{
+ match variant {
+ $(
+ $variant_pattern => $conversion,
Review Comment:
Interesting... I thought it wasn't possible for a macro to create match
arms.
But I guess it works here because the macro creates the whole match?
##########
parquet-variant-compute/src/type_conversion.rs:
##########
@@ -60,6 +65,44 @@ impl_primitive_from_variant!(datatypes::UInt64Type, as_u64);
impl_primitive_from_variant!(datatypes::Float16Type, as_f16);
impl_primitive_from_variant!(datatypes::Float32Type, as_f32);
impl_primitive_from_variant!(datatypes::Float64Type, as_f64);
+impl_primitive_from_variant!(
+ datatypes::Date32Type,
+ as_naive_date,
+ Date32Type::from_naive_date
+);
+
+pub(crate) trait TimestampFromVariant: ArrowTimestampType {
+ fn from_variant(variant: &Variant<'_, '_>) -> Option<Self::Native>;
+}
+
+macro_rules! impl_timestamp_from_variant {
+ ($timestamp_type:ty, {
+ $(($variant_pattern:pat, $conversion:expr)),+ $(,)?
+ }) => {
+ impl TimestampFromVariant for $timestamp_type {
+ fn from_variant(variant: &Variant<'_, '_>) -> Option<Self::Native>
{
+ match variant {
+ $(
+ $variant_pattern => $conversion,
+ )+
+ _ => None,
+ }
+ }
+ }
+ };
+}
+
+impl_timestamp_from_variant!(TimestampMicrosecondType, {
+ (Variant::TimestampMicros(t), Some(t.timestamp_micros())),
+ (Variant::TimestampNtzMicros(t), Some(t.and_utc().timestamp_micros())),
+});
+
+impl_timestamp_from_variant!(TimestampNanosecondType, {
+ (Variant::TimestampMicros(t), Some(t.timestamp_micros()).map(|t| t *
1000)),
+ (Variant::TimestampNtzMicros(t),
Some(t.and_utc().timestamp_micros()).map(|t| t * 1000)),
+ (Variant::TimestampNanos(t), t.timestamp_nanos_opt()),
+ (Variant::TimestampNtzNanos(t), t.and_utc().timestamp_nanos_opt()),
+});
Review Comment:
Shouldn't these be Variant::as_timestamp[_ntz]_[micros|nanos] methods?
Why defined here instead?
(defining it here means we can't just use `impl_primitive_from_variant!`
macro)
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -293,55 +324,90 @@ fn get_type_name<T: ArrowPrimitiveType>() -> &'static str
{
"arrow_array::types::Float32Type" => "Float32",
"arrow_array::types::Float64Type" => "Float64",
"arrow_array::types::Float16Type" => "Float16",
+ "arrow_array::types::TimestampMicrosecondType" =>
"Timestamp(Microsecond)",
+ "arrow_array::types::TimestampNanosecondType" =>
"Timestamp(Nanosecond)",
+ "arrow_array::types::Date32Type" => "Date32",
_ => "Unknown",
}
}
-/// Builder for converting variant values to primitive values
-pub(crate) struct VariantToPrimitiveArrowRowBuilder<'a, T:
PrimitiveFromVariant> {
- builder: arrow::array::PrimitiveBuilder<T>,
- cast_options: &'a CastOptions<'a>,
-}
-
-impl<'a, T: PrimitiveFromVariant> VariantToPrimitiveArrowRowBuilder<'a, T> {
- fn new(cast_options: &'a CastOptions<'a>, capacity: usize) -> Self {
- Self {
- builder: PrimitiveBuilder::<T>::with_capacity(capacity),
- cast_options,
+macro_rules! define_variant_to_primitive_builder {
+ (struct $name:ident<$lifetime:lifetime $(, $generic:ident: $bound:path )?>
+ |$array_param:ident $(, $field:ident: $field_type:ty)?| ->
$builder_name:ident $(< $array_type:ty >)? { $init_expr: expr },
+ |$value: ident| $value_transform:expr,
+ type_name: $type_name:expr) => {
+ pub(crate) struct $name<$lifetime $(, $generic : $bound )?>
+ {
+ builder: $builder_name $(<$array_type>)?,
+ cast_options: &$lifetime CastOptions<$lifetime>,
}
- }
-}
-impl<'a, T: PrimitiveFromVariant> VariantToPrimitiveArrowRowBuilder<'a, T> {
- fn append_null(&mut self) -> Result<()> {
- self.builder.append_null();
- Ok(())
- }
+ impl<$lifetime $(, $generic: $bound+ )?> $name<$lifetime $(, $generic
)?> {
+ fn new(
+ cast_options: &$lifetime CastOptions<$lifetime>,
+ $array_param: usize
+ // add this so that $init_expr can use it
+ $(, $field: $field_type)?) -> Self {
Review Comment:
I know nobody actually sees the emitted code, but it seems nicer to do
normal commas here?
```suggestion
$array_param: usize,
// add this so that $init_expr can use it
$( $field: $field_type, )?
) -> Self {
```
##########
parquet-variant-compute/src/type_conversion.rs:
##########
@@ -60,6 +65,44 @@ impl_primitive_from_variant!(datatypes::UInt64Type, as_u64);
impl_primitive_from_variant!(datatypes::Float16Type, as_f16);
impl_primitive_from_variant!(datatypes::Float32Type, as_f32);
impl_primitive_from_variant!(datatypes::Float64Type, as_f64);
+impl_primitive_from_variant!(
+ datatypes::Date32Type,
+ as_naive_date,
+ Date32Type::from_naive_date
+);
+
+pub(crate) trait TimestampFromVariant: ArrowTimestampType {
+ fn from_variant(variant: &Variant<'_, '_>) -> Option<Self::Native>;
+}
+
+macro_rules! impl_timestamp_from_variant {
+ ($timestamp_type:ty, {
+ $(($variant_pattern:pat, $conversion:expr)),+ $(,)?
+ }) => {
Review Comment:
I'm pretty sure this works?
```suggestion
(
$timestamp_type:ty,
$( $variant_pattern:pat => $conversion:expr ),+ $(,)?
) => {
```
Use sites would look like this:
```rust
impl_timestamp_from_variant!(
TimestampMicrosecondType,
Variant::TimestampMicros(t) => Some(t.timestamp_micros()),
Variant::TimestampNtzMicros(t) => Some(t.and_utc().timestamp_micros()),
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
