This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new c42c0a453f [Variant] Clean up redundant `get_type_name` (#8617)
c42c0a453f is described below
commit c42c0a453f48d87d071f8384dae04f00f9b31b52
Author: Liam Bao <[email protected]>
AuthorDate: Thu Oct 16 11:22:26 2025 -0400
[Variant] Clean up redundant `get_type_name` (#8617)
# Which issue does this PR close?
- Closes #8538 .
# Rationale for this change
# What changes are included in this PR?
Removes redundant `get_type_name` and leverages the `Display` trait of
`DataType`
# Are these changes tested?
Yes, 3 tests added
# Are there any user-facing changes?
No
Co-authored-by: Andrew Lamb <[email protected]>
---
parquet-variant-compute/src/variant_get.rs | 73 +++++++++++++++++++++++--
parquet-variant-compute/src/variant_to_arrow.rs | 31 ++---------
2 files changed, 74 insertions(+), 30 deletions(-)
diff --git a/parquet-variant-compute/src/variant_get.rs
b/parquet-variant-compute/src/variant_get.rs
index fda41cc84a..e782b5968a 100644
--- a/parquet-variant-compute/src/variant_get.rs
+++ b/parquet-variant-compute/src/variant_get.rs
@@ -293,12 +293,9 @@ impl<'a> GetOptions<'a> {
#[cfg(test)]
mod test {
- use std::sync::Arc;
-
use super::{GetOptions, variant_get};
- use crate::VariantArray;
- use crate::json_to_variant;
use crate::variant_array::{ShreddedVariantFieldArray, StructArrayBuilder};
+ use crate::{VariantArray, VariantArrayBuilder, json_to_variant};
use arrow::array::{
Array, ArrayRef, AsArray, BinaryViewArray, BooleanArray, Date32Array,
Decimal32Array,
Decimal64Array, Decimal128Array, Decimal256Array, Float32Array,
Float64Array, Int8Array,
@@ -315,6 +312,7 @@ mod test {
EMPTY_VARIANT_METADATA_BYTES, Variant, VariantDecimal4,
VariantDecimal8, VariantDecimal16,
VariantDecimalType, VariantPath,
};
+ use std::sync::Arc;
fn single_variant_get_test(input_json: &str, path: VariantPath,
expected_json: &str) {
// Create input array from JSON string
@@ -2104,6 +2102,73 @@ mod test {
);
}
+ #[test]
+ fn test_error_message_boolean_type_display() {
+ let mut builder = VariantArrayBuilder::new(1);
+ builder.append_variant(Variant::Int32(123));
+ let variant_array: ArrayRef = ArrayRef::from(builder.build());
+
+ // Request Boolean with strict casting to force an error
+ let options = GetOptions {
+ path: VariantPath::default(),
+ as_type: Some(Arc::new(Field::new("result", DataType::Boolean,
true))),
+ cast_options: CastOptions {
+ safe: false,
+ ..Default::default()
+ },
+ };
+
+ let err = variant_get(&variant_array, options).unwrap_err();
+ let msg = err.to_string();
+ assert!(msg.contains("Failed to extract primitive of type Boolean"));
+ }
+
+ #[test]
+ fn test_error_message_numeric_type_display() {
+ let mut builder = VariantArrayBuilder::new(1);
+ builder.append_variant(Variant::BooleanTrue);
+ let variant_array: ArrayRef = ArrayRef::from(builder.build());
+
+ // Request Boolean with strict casting to force an error
+ let options = GetOptions {
+ path: VariantPath::default(),
+ as_type: Some(Arc::new(Field::new("result", DataType::Float32,
true))),
+ cast_options: CastOptions {
+ safe: false,
+ ..Default::default()
+ },
+ };
+
+ let err = variant_get(&variant_array, options).unwrap_err();
+ let msg = err.to_string();
+ assert!(msg.contains("Failed to extract primitive of type Float32"));
+ }
+
+ #[test]
+ fn test_error_message_temporal_type_display() {
+ let mut builder = VariantArrayBuilder::new(1);
+ builder.append_variant(Variant::BooleanFalse);
+ let variant_array: ArrayRef = ArrayRef::from(builder.build());
+
+ // Request Boolean with strict casting to force an error
+ let options = GetOptions {
+ path: VariantPath::default(),
+ as_type: Some(Arc::new(Field::new(
+ "result",
+ DataType::Timestamp(TimeUnit::Nanosecond, None),
+ true,
+ ))),
+ cast_options: CastOptions {
+ safe: false,
+ ..Default::default()
+ },
+ };
+
+ let err = variant_get(&variant_array, options).unwrap_err();
+ let msg = err.to_string();
+ assert!(msg.contains("Failed to extract primitive of type
Timestamp(ns)"));
+ }
+
#[test]
fn test_null_buffer_union_for_shredded_paths() {
use arrow::compute::CastOptions;
diff --git a/parquet-variant-compute/src/variant_to_arrow.rs
b/parquet-variant-compute/src/variant_to_arrow.rs
index f2ac4a722f..9219a34be5 100644
--- a/parquet-variant-compute/src/variant_to_arrow.rs
+++ b/parquet-variant-compute/src/variant_to_arrow.rs
@@ -19,7 +19,7 @@ use arrow::array::{
ArrayRef, BinaryViewArray, BooleanBuilder, NullBufferBuilder,
PrimitiveBuilder,
};
use arrow::compute::{CastOptions, DecimalCast};
-use arrow::datatypes::{self, ArrowPrimitiveType, DataType, DecimalType};
+use arrow::datatypes::{self, DataType, DecimalType};
use arrow::error::{ArrowError, Result};
use parquet_variant::{Variant, VariantPath};
@@ -358,27 +358,6 @@ impl<'a> VariantPathRowBuilder<'a> {
}
}
-/// Helper function to get a user-friendly type name
-fn get_type_name<T: ArrowPrimitiveType>() -> &'static str {
- match std::any::type_name::<T>() {
- "arrow_array::types::Int32Type" => "Int32",
- "arrow_array::types::Int16Type" => "Int16",
- "arrow_array::types::Int8Type" => "Int8",
- "arrow_array::types::Int64Type" => "Int64",
- "arrow_array::types::UInt32Type" => "UInt32",
- "arrow_array::types::UInt16Type" => "UInt16",
- "arrow_array::types::UInt8Type" => "UInt8",
- "arrow_array::types::UInt64Type" => "UInt64",
- "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",
- }
-}
-
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 },
@@ -438,21 +417,21 @@ define_variant_to_primitive_builder!(
struct VariantToBooleanArrowRowBuilder<'a>
|capacity| -> BooleanBuilder { BooleanBuilder::with_capacity(capacity) },
|value| value.as_boolean(),
- type_name: "Boolean"
+ type_name: datatypes::BooleanType::DATA_TYPE
);
define_variant_to_primitive_builder!(
struct VariantToPrimitiveArrowRowBuilder<'a, T:PrimitiveFromVariant>
|capacity| -> PrimitiveBuilder<T> {
PrimitiveBuilder::<T>::with_capacity(capacity) },
|value| T::from_variant(value),
- type_name: get_type_name::<T>()
+ type_name: T::DATA_TYPE
);
define_variant_to_primitive_builder!(
struct VariantToTimestampNtzArrowRowBuilder<'a,
T:TimestampFromVariant<true>>
|capacity| -> PrimitiveBuilder<T> {
PrimitiveBuilder::<T>::with_capacity(capacity) },
|value| T::from_variant(value),
- type_name: get_type_name::<T>()
+ type_name: T::DATA_TYPE
);
define_variant_to_primitive_builder!(
@@ -461,7 +440,7 @@ define_variant_to_primitive_builder!(
PrimitiveBuilder::<T>::with_capacity(capacity).with_timezone_opt(tz)
},
|value| T::from_variant(value),
- type_name: get_type_name::<T>()
+ type_name: T::DATA_TYPE
);
/// Builder for converting variant values to arrow Decimal values
