Skip to content

Commit 3a15f84

Browse files
authored
[Variant] Simplify creation of Variants from metadata and value (#7663)
# Which issue does this PR close? - Part of #6736 # Rationale for this change While making documentation / examples for working with `Variant` in #7661, I found it was somewhat awkward to make `Variant` values directly from the metadata and value. Specifically you have to ```rust let metadata = [0x01, 0x00, 0x00]; let value = [0x09, 0x48, 0x49]; // parse the header metadata let metadata = VariantMetadata::try_new(&metadata).unwrap(); // and only then can you make the Variant Variant::try_new(&metadata, &value).unwrap() ``` I would really like to be able to create `Variant `directly from `metadata` and `value` without having to make a `VariantMetadata` structure # What changes are included in this PR? This PR proposes a small change to the API so creating a Variant now looks like: ```rust let metadata = [0x01, 0x00, 0x00]; let value = [0x09, 0x48, 0x49]; // You can now make the Variant directly from the metadata and value Variant::try_new(&metadata, &value).unwrap() ``` # Are there any user-facing changes? Yes, the API for creating APIs is slightly different (and I think better)
1 parent f48efc2 commit 3a15f84

File tree

2 files changed

+43
-11
lines changed

2 files changed

+43
-11
lines changed

parquet-variant/src/variant.rs

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ impl<'m> VariantMetadata<'m> {
303303

304304
#[derive(Clone, Copy, Debug, PartialEq)]
305305
pub struct VariantObject<'m, 'v> {
306-
pub metadata: &'m VariantMetadata<'m>,
306+
pub metadata: VariantMetadata<'m>,
307307
pub value_metadata: u8,
308308
pub value_data: &'v [u8],
309309
}
@@ -320,7 +320,7 @@ impl<'m, 'v> VariantObject<'m, 'v> {
320320

321321
#[derive(Clone, Copy, Debug, PartialEq)]
322322
pub struct VariantArray<'m, 'v> {
323-
pub metadata: &'m VariantMetadata<'m>,
323+
pub metadata: VariantMetadata<'m>,
324324
pub value_metadata: u8,
325325
pub value_data: &'v [u8],
326326
}
@@ -389,7 +389,7 @@ impl<'m, 'v> VariantArray<'m, 'v> {
389389
first_value_byte + start_field_offset_from_first_value_byte
390390
..first_value_byte + end_field_offset_from_first_value_byte,
391391
)?;
392-
let variant = Variant::try_new(self.metadata, variant_value_bytes)?;
392+
let variant = Variant::try_new_with_metadata(self.metadata, variant_value_bytes)?;
393393
Ok(variant)
394394
}
395395
}
@@ -430,8 +430,41 @@ pub enum Variant<'m, 'v> {
430430
}
431431

432432
impl<'m, 'v> Variant<'m, 'v> {
433-
/// Parse the buffers and return the appropriate variant.
434-
pub fn try_new(metadata: &'m VariantMetadata, value: &'v [u8]) -> Result<Self, ArrowError> {
433+
/// Create a new `Variant` from metadata and value.
434+
///
435+
/// # Example
436+
/// ```
437+
/// # use parquet_variant::{Variant, VariantMetadata};
438+
/// let metadata = [0x01, 0x00, 0x00];
439+
/// let value = [0x09, 0x48, 0x49];
440+
/// assert_eq!(
441+
/// Variant::ShortString("HI"),
442+
/// Variant::try_new(&metadata, &value).unwrap()
443+
/// );
444+
/// ```
445+
pub fn try_new(metadata: &'m [u8], value: &'v [u8]) -> Result<Self, ArrowError> {
446+
let metadata = VariantMetadata::try_new(metadata)?;
447+
Self::try_new_with_metadata(metadata, value)
448+
}
449+
450+
/// Create a new variant with existing metadata
451+
///
452+
/// # Example
453+
/// ```
454+
/// # use parquet_variant::{Variant, VariantMetadata};
455+
/// let metadata = [0x01, 0x00, 0x00];
456+
/// let value = [0x09, 0x48, 0x49];
457+
/// // parse the header metadata first
458+
/// let metadata = VariantMetadata::try_new(&metadata).unwrap();
459+
/// assert_eq!(
460+
/// Variant::ShortString("HI"),
461+
/// Variant::try_new_with_metadata(metadata, &value).unwrap()
462+
/// );
463+
/// ```
464+
pub fn try_new_with_metadata(
465+
metadata: VariantMetadata<'m>,
466+
value: &'v [u8],
467+
) -> Result<Self, ArrowError> {
435468
let value_metadata = *first_byte_from_slice(value)?;
436469
let value_data = slice_from_slice(value, 1..)?;
437470
let new_self = match get_basic_type(value_metadata)? {
@@ -995,7 +1028,7 @@ impl<'m, 'v> Variant<'m, 'v> {
9951028
pub fn metadata(&self) -> Option<&'m VariantMetadata> {
9961029
match self {
9971030
Variant::Object(VariantObject { metadata, .. })
998-
| Variant::Array(VariantArray { metadata, .. }) => Some(*metadata),
1031+
| Variant::Array(VariantArray { metadata, .. }) => Some(metadata),
9991032
_ => None,
10001033
}
10011034
}

parquet-variant/tests/variant_interop.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ fn get_non_primitive_cases() -> Vec<&'static str> {
7676
fn variant_primitive() -> Result<(), ArrowError> {
7777
let cases = get_primitive_cases();
7878
for (case, want) in cases {
79-
let (metadata_bytes, value) = load_case(case)?;
80-
let metadata = VariantMetadata::try_new(&metadata_bytes)?;
79+
let (metadata, value) = load_case(case)?;
8180
let got = Variant::try_new(&metadata, &value)?;
8281
assert_eq!(got, want);
8382
}
@@ -89,13 +88,13 @@ fn variant_non_primitive() -> Result<(), ArrowError> {
8988
let cases = get_non_primitive_cases();
9089
for case in cases {
9190
let (metadata, value) = load_case(case)?;
92-
let metadata = VariantMetadata::try_new(&metadata)?;
91+
let variant_metadata = VariantMetadata::try_new(&metadata)?;
9392
let variant = Variant::try_new(&metadata, &value)?;
9493
match case {
9594
"object_primitive" => {
9695
assert!(matches!(variant, Variant::Object(_)));
97-
assert_eq!(metadata.dictionary_size(), 7);
98-
let dict_val = metadata.get_field_by(0)?;
96+
assert_eq!(variant_metadata.dictionary_size(), 7);
97+
let dict_val = variant_metadata.get_field_by(0)?;
9998
assert_eq!(dict_val, "int_field");
10099
}
101100
"array_primitive" => match variant {

0 commit comments

Comments
 (0)