From 3f8cf0d566c0a3debeaa72945c7db4dd68f0d7ae Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 3 Oct 2026 10:17:39 +0200 Subject: [PATCH 1/8] Benchmark Arrow JSON deserialization alongside direct APIs --- Readme.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Readme.md b/Readme.md index be66acf4..fe033d35 100644 --- a/Readme.md +++ b/Readme.md @@ -128,7 +128,13 @@ benchmark results are workload-specific and only indicative. ![Serialization and deserialization runtimes relative to their baselines](timings.png) -The detailed runtimes of the [benchmarks](serde_arrow_bench/benches/groups/) are listed below. +The tables below give runtimes and pairwise ratios for the +[benchmark workloads](serde_arrow_bench/benches/groups/). +Deserialization benchmarks decode 1,000 records from arrays prepared before timing. The +`serde_arrow` calls include deserializer setup and creation of owned Rust records. The manual +baseline reads the same Arrow arrays and builds the same records directly. The Arrow JSON +comparison writes the arrays to JSON with `arrow_json::WriterBuilder` and then parses them +into the same Rust records with `serde_json`. ### Serialization From 9a0fe794f03fec2cfadebf9cca3ff41a36ee632a Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 3 Oct 2026 10:25:07 +0200 Subject: [PATCH 2/8] Simplify readme --- Readme.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Readme.md b/Readme.md index fe033d35..be66acf4 100644 --- a/Readme.md +++ b/Readme.md @@ -128,13 +128,7 @@ benchmark results are workload-specific and only indicative. ![Serialization and deserialization runtimes relative to their baselines](timings.png) -The tables below give runtimes and pairwise ratios for the -[benchmark workloads](serde_arrow_bench/benches/groups/). -Deserialization benchmarks decode 1,000 records from arrays prepared before timing. The -`serde_arrow` calls include deserializer setup and creation of owned Rust records. The manual -baseline reads the same Arrow arrays and builds the same records directly. The Arrow JSON -comparison writes the arrays to JSON with `arrow_json::WriterBuilder` and then parses them -into the same Rust records with `serde_json`. +The detailed runtimes of the [benchmarks](serde_arrow_bench/benches/groups/) are listed below. ### Serialization From 0c97f0f76eb98cbf2ace5d0b4f94231e61a11f5c Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 26 Sep 2026 20:17:38 +0200 Subject: [PATCH 3/8] Speed up deserialization without API changes --- Cargo.lock | 1 + serde_arrow/Cargo.toml | 1 + .../deserialization/list_deserializer.rs | 4 +++ .../deserialization/struct_deserializer.rs | 20 +++++++++--- serde_arrow/src/internal/deserializer.rs | 11 +++++++ .../src/internal/utils/array_view_ext.rs | 31 ++++++++++++++++++- 6 files changed, 63 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 36895b91..71f283e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1840,6 +1840,7 @@ dependencies = [ "serde", "serde_bytes", "serde_json", + "simdutf8", "uuid", ] diff --git a/serde_arrow/Cargo.toml b/serde_arrow/Cargo.toml index cdb0cb5f..154ce5bb 100644 --- a/serde_arrow/Cargo.toml +++ b/serde_arrow/Cargo.toml @@ -37,6 +37,7 @@ bytemuck = { version = "1", default-features = false } chrono = { version = "0.4", features = ["std"], default-features = false } half = { version = "2", features = ["bytemuck"], default-features = false } serde = { version = "1.0", features = ["derive", "std"], default-features = false } +simdutf8 = { version = "0.1", default-features = false } # arrow-version:insert: arrow-array-{version} = {{ package = "arrow-array", version = "{version}", optional = true, default-features = false }} arrow-array-60 = { package = "arrow-array", version = "60", optional = true, default-features = false } diff --git a/serde_arrow/src/internal/deserialization/list_deserializer.rs b/serde_arrow/src/internal/deserialization/list_deserializer.rs index 1df8665f..c0e2ce89 100644 --- a/serde_arrow/src/internal/deserialization/list_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/list_deserializer.rs @@ -131,4 +131,8 @@ impl<'de> SeqAccess<'de> for ListItemDeserializer<'_, 'de> { self.start += 1; Ok(Some(item)) } + + fn size_hint(&self) -> Option { + Some(self.end.saturating_sub(self.start).min(1024)) + } } diff --git a/serde_arrow/src/internal/deserialization/struct_deserializer.rs b/serde_arrow/src/internal/deserialization/struct_deserializer.rs index 743e65e8..24c72a13 100644 --- a/serde_arrow/src/internal/deserialization/struct_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/struct_deserializer.rs @@ -93,13 +93,25 @@ impl<'de> RandomAccessDeserializer<'de> for StructDeserializer<'de> { fn deserialize_struct>( &self, _: &'static str, - _: &'static [&'static str], + fields: &'static [&'static str], visitor: V, idx: usize, ) -> Result { - visitor - .visit_map(StructItemDeserializer::new(self, idx)) - .ctx(self) + if self.fields.len() == fields.len() + && self + .fields + .iter() + .zip(fields) + .all(|((actual, _), expected)| actual == expected) + { + visitor + .visit_seq(StructItemDeserializer::new(self, idx)) + .ctx(self) + } else { + visitor + .visit_map(StructItemDeserializer::new(self, idx)) + .ctx(self) + } } fn deserialize_tuple>( diff --git a/serde_arrow/src/internal/deserializer.rs b/serde_arrow/src/internal/deserializer.rs index 569d7cb6..b24449de 100644 --- a/serde_arrow/src/internal/deserializer.rs +++ b/serde_arrow/src/internal/deserializer.rs @@ -530,6 +530,17 @@ impl<'de> SeqAccess<'de> for Private> { self.0.next += 1; Ok(Some(item)) } + + fn size_hint(&self) -> Option { + // Keep preallocation bounded when views come from untrusted Marrow offsets. + Some( + self.0 + .deserializer + .len + .saturating_sub(self.0.next) + .min(1024), + ) + } } #[allow(unused, reason = "trait assertion")] diff --git a/serde_arrow/src/internal/utils/array_view_ext.rs b/serde_arrow/src/internal/utils/array_view_ext.rs index 810d5b19..92bfd4b6 100644 --- a/serde_arrow/src/internal/utils/array_view_ext.rs +++ b/serde_arrow/src/internal/utils/array_view_ext.rs @@ -231,9 +231,38 @@ where { fn get(&self, idx: usize) -> Result> { match ViewAccess::<[u8]>::get(self, idx) { - Ok(Some(data)) => Ok(Some(std::str::from_utf8(data)?)), + // Preserve the detailed std error for malformed Marrow views. + Ok(Some(data)) => match simdutf8::basic::from_utf8(data) { + Ok(value) => Ok(Some(value)), + Err(_) => Ok(Some(std::str::from_utf8(data)?)), + }, Ok(None) => Ok(None), Err(err) => Err(err), } } } + +#[cfg(test)] +mod tests { + use marrow::view::BytesView; + + use super::ViewAccess; + + #[test] + fn utf8_access_validates_marrow_data() { + let valid = BytesView { + offsets: &[0_i32, 5], + data: "café".as_bytes(), + validity: None, + }; + assert_eq!(ViewAccess::::get(&valid, 0).unwrap(), Some("café")); + + let invalid = BytesView { + offsets: &[0_i32, 2], + data: &[b'a', 0xff], + validity: None, + }; + let error = ViewAccess::::get(&invalid, 0).unwrap_err(); + assert!(error.to_string().contains("invalid UTF-8 data")); + } +} From 7e74b8cbec2e93f1b93050bde1fc64e075fad7a3 Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 26 Sep 2026 22:06:53 +0200 Subject: [PATCH 4/8] Cache matching struct field order during deserialization --- .../deserialization/struct_deserializer.rs | 100 ++++++++++++++++-- 1 file changed, 93 insertions(+), 7 deletions(-) diff --git a/serde_arrow/src/internal/deserialization/struct_deserializer.rs b/serde_arrow/src/internal/deserialization/struct_deserializer.rs index 24c72a13..fe9181bb 100644 --- a/serde_arrow/src/internal/deserialization/struct_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/struct_deserializer.rs @@ -1,3 +1,5 @@ +use std::sync::atomic::{AtomicPtr, Ordering}; + use marrow::view::{BitsWithOffset, StructView}; use serde::de::{value::StrDeserializer, DeserializeSeed, MapAccess, SeqAccess, Visitor}; @@ -17,6 +19,7 @@ pub struct StructDeserializer<'a> { pub fields: Vec<(String, ArrayDeserializer<'a>)>, pub validity: Option>, pub len: usize, + ordered_fields: AtomicPtr<&'static str>, } impl<'a> StructDeserializer<'a> { @@ -48,6 +51,32 @@ impl<'a> StructDeserializer<'a> { fields, validity, len, + ordered_fields: AtomicPtr::new(std::ptr::null_mut()), + } + } + + fn fields_match(&self, fields: &'static [&'static str]) -> bool { + if self.fields.len() != fields.len() { + return false; + } + + // Derived Deserialize implementations use a static field slice. Cache the + // matching slice's address so subsequent rows skip string comparisons. + let field_ptr = fields.as_ptr() as *mut &'static str; + if self.ordered_fields.load(Ordering::Relaxed) == field_ptr { + return true; + } + + if self + .fields + .iter() + .zip(fields) + .all(|((actual, _), expected)| actual == expected) + { + self.ordered_fields.store(field_ptr, Ordering::Relaxed); + true + } else { + false } } } @@ -97,13 +126,7 @@ impl<'de> RandomAccessDeserializer<'de> for StructDeserializer<'de> { visitor: V, idx: usize, ) -> Result { - if self.fields.len() == fields.len() - && self - .fields - .iter() - .zip(fields) - .all(|((actual, _), expected)| actual == expected) - { + if self.fields_match(fields) { visitor .visit_seq(StructItemDeserializer::new(self, idx)) .ctx(self) @@ -195,3 +218,66 @@ impl<'de> SeqAccess<'de> for StructItemDeserializer<'_, 'de> { Ok(Some(res)) } } + +#[cfg(test)] +mod tests { + use marrow::{ + datatypes::{DataType, Field}, + view::{PrimitiveView, View}, + }; + use serde::Deserialize; + + use crate::Deserializer; + + #[derive(Debug, Deserialize, PartialEq)] + struct Ordered { + a: i32, + b: i32, + } + + #[derive(Debug, Deserialize, PartialEq)] + struct Reordered { + b: i32, + a: i32, + } + + #[test] + fn cached_field_match_preserves_reordered_deserialization() { + let fields = [ + Field { + name: "a".into(), + data_type: DataType::Int32, + ..Field::default() + }, + Field { + name: "b".into(), + data_type: DataType::Int32, + ..Field::default() + }, + ]; + let views = [ + View::Int32(PrimitiveView { + values: &[11, 33], + validity: None, + }), + View::Int32(PrimitiveView { + values: &[22, 44], + validity: None, + }), + ]; + let deserializer = Deserializer::from_marrow(&fields, &views).unwrap(); + + assert_eq!( + Ordered::deserialize(deserializer.get(0).unwrap()).unwrap(), + Ordered { a: 11, b: 22 } + ); + assert_eq!( + Reordered::deserialize(deserializer.get(0).unwrap()).unwrap(), + Reordered { a: 11, b: 22 } + ); + assert_eq!( + Ordered::deserialize(deserializer.get(1).unwrap()).unwrap(), + Ordered { a: 33, b: 44 } + ); + } +} From 94252a88ab39cba6bbf964041e5aedd1b1cdc96a Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 26 Sep 2026 22:36:00 +0200 Subject: [PATCH 5/8] Skip redundant UTF-8 checks for Arrow deserialization --- serde_arrow/src/arrow_impl.rs | 139 +++++++++++++++++- .../deserialization/array_deserializer.rs | 84 +++++++---- .../dictionary_deserializer.rs | 14 +- .../deserialization/enum_deserializer.rs | 6 +- .../fixed_size_list_deserializer.rs | 11 +- .../deserialization/list_deserializer.rs | 11 +- .../deserialization/map_deserializer.rs | 10 +- .../src/internal/deserialization/mod.rs | 20 +++ .../deserialization/string_deserializer.rs | 53 +++++-- .../deserialization/struct_deserializer.rs | 11 +- serde_arrow/src/internal/deserializer.rs | 12 +- .../test_with_arrow/impls/arrow_dictionary.rs | 48 ++++-- 12 files changed, 352 insertions(+), 67 deletions(-) diff --git a/serde_arrow/src/arrow_impl.rs b/serde_arrow/src/arrow_impl.rs index 21e5627f..3827d8d5 100644 --- a/serde_arrow/src/arrow_impl.rs +++ b/serde_arrow/src/arrow_impl.rs @@ -15,6 +15,7 @@ use crate::{ }, internal::{ array_builder::ArrayBuilder, + deserialization::Utf8Validation, deserializer::Deserializer, error::{fail, Error, ErrorKind, Result}, schema::extensions::{Bool8Field, FixedShapeTensorField, VariableShapeTensorField}, @@ -281,7 +282,11 @@ impl<'de> Deserializer<'de> { views.push(View::try_from(array.as_ref())?); } - Deserializer::new(&fields, views) + // SAFETY: each view was converted from an Arrow array. Arrow's unsafe + // Array contract requires UTF-8 string values to be valid, including + // values held by nested arrays and dictionaries. + let utf8_validation = unsafe { Utf8Validation::trusted_arrow() }; + Deserializer::new_with_utf8_validation(&fields, views, utf8_validation) } /// Construct a new deserializer from a record batch (*requires one of the @@ -446,3 +451,135 @@ macro_rules! impl_try_from_ext_type { impl_try_from_ext_type!(Bool8Field); impl_try_from_ext_type!(FixedShapeTensorField); impl_try_from_ext_type!(VariableShapeTensorField); + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use serde::{Deserialize, Serialize}; + + use crate::{ + _impl::arrow::datatypes::{DataType, Field as ArrowField, FieldRef}, + schema::{SchemaLike, TracingOptions}, + }; + + use super::{from_arrow, to_arrow}; + + #[derive(Debug, Deserialize, Serialize, PartialEq)] + struct OptionalString { + value: Option, + } + + #[derive(Debug, Deserialize, PartialEq)] + struct BorrowedString<'a> { + #[serde(borrow)] + value: Option<&'a str>, + } + + #[test] + fn trusted_arrow_string_types() { + let expected = vec![ + OptionalString { + value: Some("café".into()), + }, + OptionalString { value: None }, + OptionalString { + value: Some("a string longer than twelve bytes".into()), + }, + ]; + + for data_type in [DataType::Utf8, DataType::LargeUtf8, DataType::Utf8View] { + let fields = vec![Arc::new(ArrowField::new("value", data_type, true))]; + let arrays = to_arrow(&fields, &expected).unwrap(); + let actual: Vec = from_arrow(&fields, &arrays).unwrap(); + assert_eq!(actual, expected); + let borrowed: Vec> = from_arrow(&fields, &arrays).unwrap(); + assert_eq!(borrowed[0].value, Some("café")); + assert_eq!(borrowed[1].value, None); + } + } + + #[derive(Debug, Deserialize, Serialize, PartialEq)] + struct NestedStrings { + child: Child, + strings: Vec, + } + + #[derive(Debug, Deserialize, Serialize, PartialEq)] + struct Child { + label: String, + } + + #[test] + fn trusted_arrow_nested_strings() { + let items = vec![NestedStrings { + child: Child { + label: "éclair".into(), + }, + strings: vec!["one".into(), "zürich".into()], + }]; + let fields = + Vec::::from_type::(TracingOptions::default()).unwrap(); + let arrays = to_arrow(&fields, &items).unwrap(); + let actual: Vec = from_arrow(&fields, &arrays).unwrap(); + assert_eq!(actual, items); + } + + #[derive(Debug, Deserialize, PartialEq)] + struct RequiredString { + value: String, + } + + #[test] + fn trusted_arrow_dictionary_strings() { + use marrow::array::{Array as MarrowArray, BytesArray, DictionaryArray, PrimitiveArray}; + + let dictionary = MarrowArray::Dictionary(DictionaryArray { + keys: Box::new(MarrowArray::Int8(PrimitiveArray { + values: vec![1, 0], + validity: None, + })), + values: Box::new(MarrowArray::Utf8(BytesArray { + offsets: vec![0, 5, 12], + data: "cafézürich".as_bytes().to_vec(), + validity: None, + })), + }); + let array = crate::_impl::arrow::array::ArrayRef::try_from(dictionary).unwrap(); + let fields = vec![Arc::new(ArrowField::new( + "value", + array.data_type().clone(), + false, + ))]; + + let actual: Vec = from_arrow(&fields, &[array]).unwrap(); + assert_eq!( + actual, + vec![ + RequiredString { + value: "zürich".into() + }, + RequiredString { + value: "café".into() + }, + ] + ); + } + + #[test] + fn marrow_strings_still_validate_utf8() { + let fields = vec![marrow::datatypes::Field { + name: "value".into(), + data_type: marrow::datatypes::DataType::Utf8, + ..marrow::datatypes::Field::default() + }]; + let views = [marrow::view::View::Utf8(marrow::view::BytesView { + offsets: &[0, 1], + data: &[0xff], + validity: None, + })]; + + let result = crate::from_marrow::>(&fields, &views); + assert!(result.is_err()); + } +} diff --git a/serde_arrow/src/internal/deserialization/array_deserializer.rs b/serde_arrow/src/internal/deserialization/array_deserializer.rs index e4ac7727..3e9f6ea7 100644 --- a/serde_arrow/src/internal/deserialization/array_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/array_deserializer.rs @@ -30,6 +30,7 @@ use super::{ struct_deserializer::StructDeserializer, time_deserializer::TimeDeserializer, timestamp_deserializer::TimestampDeserializer, + Utf8Validation, }; pub enum ArrayDeserializer<'a> { @@ -86,7 +87,12 @@ pub enum ArrayDeserializer<'a> { impl<'a> ArrayDeserializer<'a> { // TODO: decide whether to keep strategy parameter - pub fn new(path: String, _strategy: Option<&Strategy>, array: View<'a>) -> Result { + pub fn new_with_utf8_validation( + path: String, + _strategy: Option<&Strategy>, + array: View<'a>, + utf8_validation: Utf8Validation, + ) -> Result { use {ArrayDeserializer as D, View as V}; match array { View::Null(_) => Ok(Self::Null(NullDeserializer::new(path))), @@ -109,71 +115,97 @@ impl<'a> ArrayDeserializer<'a> { V::Time64(view) => Ok(D::Time64(TimeDeserializer::new(path, view))), V::Timestamp(view) => Ok(Self::Timestamp(TimestampDeserializer::new(path, view)?)), V::Duration(view) => Ok(D::Duration(DurationDeserializer::new(path, view))), - V::Utf8(view) => Ok(D::Utf8(StringDeserializer::new(path, view))), - V::LargeUtf8(view) => Ok(D::LargeUtf8(StringDeserializer::new(path, view))), - V::Utf8View(view) => Ok(D::Utf8View(StringDeserializer::new(path, view))), + V::Utf8(view) => Ok(D::Utf8(StringDeserializer::new( + path, + view, + utf8_validation, + ))), + V::LargeUtf8(view) => Ok(D::LargeUtf8(StringDeserializer::new( + path, + view, + utf8_validation, + ))), + V::Utf8View(view) => Ok(D::Utf8View(StringDeserializer::new( + path, + view, + utf8_validation, + ))), V::Binary(view) => Ok(D::Binary(BinaryDeserializer::new(path, view))), V::LargeBinary(view) => Ok(D::LargeBinary(BinaryDeserializer::new(path, view))), V::BinaryView(view) => Ok(D::BinaryView(BinaryDeserializer::new(path, view))), V::FixedSizeBinary(view) => Ok(D::FixedSizeBinary(FixedSizeBinaryDeserializer::new( path, view, )?)), - V::List(view) => Ok(D::List(ListDeserializer::new(path, view)?)), - V::LargeList(view) => Ok(D::LargeList(ListDeserializer::new(path, view)?)), + V::List(view) => Ok(D::List(ListDeserializer::new(path, view, utf8_validation)?)), + V::LargeList(view) => Ok(D::LargeList(ListDeserializer::new( + path, + view, + utf8_validation, + )?)), V::FixedSizeList(view) => Ok(D::FixedSizeList(FixedSizeListDeserializer::new( - path, view, + path, + view, + utf8_validation, + )?)), + V::Struct(view) => Ok(D::Struct(StructDeserializer::new( + path, + view, + utf8_validation, + )?)), + V::Map(view) => Ok(D::Map(MapDeserializer::new(path, view, utf8_validation)?)), + View::Union(view) => Ok(Self::Enum(EnumDeserializer::new( + path, + view, + utf8_validation, )?)), - V::Struct(view) => Ok(D::Struct(StructDeserializer::new(path, view)?)), - V::Map(view) => Ok(D::Map(MapDeserializer::new(path, view)?)), - View::Union(view) => Ok(Self::Enum(EnumDeserializer::new(path, view)?)), V::Dictionary(view) => match (*view.keys, *view.values) { (V::Int8(keys), V::Utf8(values)) => Ok(D::DictionaryI8I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int16(keys), V::Utf8(values)) => Ok(D::DictionaryI16I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int32(keys), V::Utf8(values)) => Ok(D::DictionaryI32I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int64(keys), V::Utf8(values)) => Ok(D::DictionaryI64I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt8(keys), V::Utf8(values)) => Ok(Self::DictionaryU8I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt16(keys), V::Utf8(values)) => Ok(D::DictionaryU16I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt32(keys), V::Utf8(values)) => Ok(D::DictionaryU32I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt64(keys), V::Utf8(values)) => Ok(D::DictionaryU64I32( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int8(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI8I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int16(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI16I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int32(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI32I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::Int64(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI64I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt8(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU8I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt16(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU16I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt32(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU32I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), (V::UInt64(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU64I64( - DictionaryDeserializer::new(path, keys, values)?, + DictionaryDeserializer::new(path, keys, values, utf8_validation)?, )), _ => fail!("unsupported dictionary array type"), }, diff --git a/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs b/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs index 3c6158b1..e0e7971b 100644 --- a/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs @@ -8,27 +8,35 @@ use crate::internal::{ use super::{ enums_as_string_impl::EnumAccess, integer_deserializer::Integer, - random_access_deserializer::RandomAccessDeserializer, + random_access_deserializer::RandomAccessDeserializer, string_deserializer::get_utf8, + Utf8Validation, }; pub struct DictionaryDeserializer<'a, K: Integer, V: Offset> { path: String, keys: PrimitiveView<'a, K>, values: BytesView<'a, V>, + utf8_validation: Utf8Validation, } impl<'a, K: Integer, V: Offset> DictionaryDeserializer<'a, K, V> { - pub fn new(path: String, keys: PrimitiveView<'a, K>, values: BytesView<'a, V>) -> Result { + pub fn new( + path: String, + keys: PrimitiveView<'a, K>, + values: BytesView<'a, V>, + utf8_validation: Utf8Validation, + ) -> Result { Ok(Self { path, keys: keys.clone(), values: values.clone(), + utf8_validation, }) } pub fn get_str(&self, idx: usize) -> Result<&str> { let key: usize = self.keys.get_required(idx)?.into_i64()?.try_into()?; - let value: &str = self.values.get_required(key)?; + let value: &str = get_utf8(&self.values, key, self.utf8_validation)?; Ok(value) } } diff --git a/serde_arrow/src/internal/deserialization/enum_deserializer.rs b/serde_arrow/src/internal/deserialization/enum_deserializer.rs index e35f0ea0..6e4daec6 100644 --- a/serde_arrow/src/internal/deserialization/enum_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/enum_deserializer.rs @@ -10,6 +10,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::{PositionedDeserializer, RandomAccessDeserializer}, + Utf8Validation, }; pub struct EnumDeserializer<'a> { @@ -20,7 +21,7 @@ pub struct EnumDeserializer<'a> { } impl<'a> EnumDeserializer<'a> { - pub fn new(path: String, view: UnionView<'a>) -> Result { + pub fn new(path: String, view: UnionView<'a>, utf8_validation: Utf8Validation) -> Result { let Some(offsets) = view.offsets else { fail!("only dense unions are supported"); }; @@ -36,10 +37,11 @@ impl<'a> EnumDeserializer<'a> { fail!("only unions with consecutive type ids are currently supported"); } let child_path = format!("{path}.{child}", child = ChildName(&field_meta.name)); - let field_deserializer = ArrayDeserializer::new( + let field_deserializer = ArrayDeserializer::new_with_utf8_validation( child_path, get_strategy_from_metadata(&field_meta.metadata)?.as_ref(), field_view, + utf8_validation, )?; variants.push((field_meta.name, field_deserializer)) } diff --git a/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs b/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs index 6d870f8c..c1f06b93 100644 --- a/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, list_deserializer::ListItemDeserializer, - random_access_deserializer::RandomAccessDeserializer, utils::bitset_is_set, + random_access_deserializer::RandomAccessDeserializer, utils::bitset_is_set, Utf8Validation, }; pub struct FixedSizeListDeserializer<'a> { @@ -21,12 +21,17 @@ pub struct FixedSizeListDeserializer<'a> { } impl<'a> FixedSizeListDeserializer<'a> { - pub fn new(path: String, view: FixedSizeListView<'a>) -> Result { + pub fn new( + path: String, + view: FixedSizeListView<'a>, + utf8_validation: Utf8Validation, + ) -> Result { let child_path = format!("{path}.{child}", child = ChildName(&view.meta.name)); - let item = ArrayDeserializer::new( + let item = ArrayDeserializer::new_with_utf8_validation( child_path, get_strategy_from_metadata(&view.meta.metadata)?.as_ref(), *view.elements, + utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/list_deserializer.rs b/serde_arrow/src/internal/deserialization/list_deserializer.rs index c0e2ce89..72f687f7 100644 --- a/serde_arrow/src/internal/deserialization/list_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/list_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, + utils::bitset_is_set, Utf8Validation, }; pub struct ListDeserializer<'a, O: Offset> { @@ -20,12 +20,17 @@ pub struct ListDeserializer<'a, O: Offset> { } impl<'de, O: Offset> ListDeserializer<'de, O> { - pub fn new(path: String, view: ListView<'de, O>) -> Result { + pub fn new( + path: String, + view: ListView<'de, O>, + utf8_validation: Utf8Validation, + ) -> Result { let child_path = format!("{path}.{child}", child = ChildName(&view.meta.name)); - let item = ArrayDeserializer::new( + let item = ArrayDeserializer::new_with_utf8_validation( child_path, get_strategy_from_metadata(&view.meta.metadata)?.as_ref(), *view.elements, + utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/map_deserializer.rs b/serde_arrow/src/internal/deserialization/map_deserializer.rs index 044d54b2..63cfbc88 100644 --- a/serde_arrow/src/internal/deserialization/map_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/map_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, + utils::bitset_is_set, Utf8Validation, }; pub struct MapDeserializer<'a> { @@ -21,16 +21,17 @@ pub struct MapDeserializer<'a> { } impl<'a> MapDeserializer<'a> { - pub fn new(path: String, view: MapView<'a>) -> Result { + pub fn new(path: String, view: MapView<'a>, utf8_validation: Utf8Validation) -> Result { let keys_path = format!( "{path}.{entries}.{keys}", entries = ChildName(&view.meta.entries_name), keys = ChildName(&view.meta.keys.name), ); - let keys = ArrayDeserializer::new( + let keys = ArrayDeserializer::new_with_utf8_validation( keys_path, get_strategy_from_metadata(&view.meta.keys.metadata)?.as_ref(), *view.keys, + utf8_validation, )?; let values_path = format!( @@ -38,10 +39,11 @@ impl<'a> MapDeserializer<'a> { entries = ChildName(&view.meta.entries_name), values = ChildName(&view.meta.values.name), ); - let values = ArrayDeserializer::new( + let values = ArrayDeserializer::new_with_utf8_validation( values_path, get_strategy_from_metadata(&view.meta.values.metadata)?.as_ref(), *view.values, + utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/mod.rs b/serde_arrow/src/internal/deserialization/mod.rs index c63e2ab0..bc30d4e2 100644 --- a/serde_arrow/src/internal/deserialization/mod.rs +++ b/serde_arrow/src/internal/deserialization/mod.rs @@ -21,3 +21,23 @@ pub mod struct_deserializer; pub mod time_deserializer; pub mod timestamp_deserializer; pub mod utils; + +#[derive(Clone, Copy)] +pub(crate) struct Utf8Validation { + trusted: bool, +} + +impl Utf8Validation { + pub(crate) const CHECKED: Self = Self { trusted: false }; + + /// # Safety + /// Every string view used with this marker must originate from a valid Arrow + /// string array, including nested and dictionary values. + pub(crate) unsafe fn trusted_arrow() -> Self { + Self { trusted: true } + } + + pub(crate) fn is_trusted(self) -> bool { + self.trusted + } +} diff --git a/serde_arrow/src/internal/deserialization/string_deserializer.rs b/serde_arrow/src/internal/deserialization/string_deserializer.rs index 6f27fbf3..2751cc1d 100644 --- a/serde_arrow/src/internal/deserialization/string_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/string_deserializer.rs @@ -7,6 +7,7 @@ use crate::internal::{ use super::{ enums_as_string_impl::EnumAccess, random_access_deserializer::RandomAccessDeserializer, + Utf8Validation, }; pub trait StringDeserializerDataType { @@ -28,11 +29,30 @@ impl StringDeserializerDataType for BytesViewView<'_> { pub struct StringDeserializer { pub path: String, pub view: V, + utf8_validation: Utf8Validation, } impl StringDeserializer { - pub fn new(path: String, view: V) -> Self { - Self { path, view } + pub fn new(path: String, view: V, utf8_validation: Utf8Validation) -> Self { + Self { + path, + view, + utf8_validation, + } + } +} + +pub(super) fn get_utf8<'a, V>(view: &V, idx: usize, validation: Utf8Validation) -> Result<&'a str> +where + V: ViewAccess<'a, str> + ViewAccess<'a, [u8]> + 'a, +{ + if validation.is_trusted() { + let bytes = ViewAccess::<[u8]>::get_required(view, idx)?; + // SAFETY: the trusted marker requires all string views to come from + // valid Arrow arrays. Their unsafe Array contract guarantees UTF-8. + Ok(unsafe { std::str::from_utf8_unchecked(bytes) }) + } else { + ViewAccess::::get_required(view, idx) } } @@ -45,10 +65,14 @@ impl Context for StringDeserializer { impl<'a, VV> RandomAccessDeserializer<'a> for StringDeserializer where - VV: ViewAccess<'a, str> + StringDeserializerDataType + 'a, + VV: ViewAccess<'a, str> + ViewAccess<'a, [u8]> + StringDeserializerDataType + 'a, { fn is_some(&self, idx: usize) -> Result { - self.view.is_some(idx) + if self.utf8_validation.is_trusted() { + ViewAccess::<[u8]>::is_some(&self.view, idx) + } else { + ViewAccess::::is_some(&self.view, idx) + } } fn deserialize_any_some>( @@ -64,7 +88,8 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_borrowed_str(self.view.get_required(idx)?)).ctx(self) + try_(|| visitor.visit_borrowed_str(get_utf8(&self.view, idx, self.utf8_validation)?)) + .ctx(self) } fn deserialize_string>( @@ -72,7 +97,8 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_string(self.view.get_required(idx)?.to_owned())).ctx(self) + try_(|| visitor.visit_string(get_utf8(&self.view, idx, self.utf8_validation)?.to_owned())) + .ctx(self) } fn deserialize_bytes>( @@ -80,7 +106,8 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_bytes(self.view.get_required(idx)?.as_bytes())).ctx(self) + try_(|| visitor.visit_bytes(get_utf8(&self.view, idx, self.utf8_validation)?.as_bytes())) + .ctx(self) } fn deserialize_byte_buf>( @@ -88,8 +115,14 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_byte_buf(self.view.get_required(idx)?.to_owned().into_bytes())) - .ctx(self) + try_(|| { + visitor.visit_byte_buf( + get_utf8(&self.view, idx, self.utf8_validation)? + .to_owned() + .into_bytes(), + ) + }) + .ctx(self) } fn deserialize_enum>( @@ -100,7 +133,7 @@ where idx: usize, ) -> Result { try_(|| { - let variant = self.view.get_required(idx)?; + let variant = get_utf8(&self.view, idx, self.utf8_validation)?; visitor.visit_enum(EnumAccess(variant)) }) .ctx(self) diff --git a/serde_arrow/src/internal/deserialization/struct_deserializer.rs b/serde_arrow/src/internal/deserialization/struct_deserializer.rs index fe9181bb..271a5078 100644 --- a/serde_arrow/src/internal/deserialization/struct_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/struct_deserializer.rs @@ -11,7 +11,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, + utils::bitset_is_set, Utf8Validation, }; pub struct StructDeserializer<'a> { @@ -23,14 +23,19 @@ pub struct StructDeserializer<'a> { } impl<'a> StructDeserializer<'a> { - pub fn new(path: String, view: StructView<'a>) -> Result { + pub fn new( + path: String, + view: StructView<'a>, + utf8_validation: Utf8Validation, + ) -> Result { let mut fields = Vec::with_capacity(view.fields.len()); for (field_meta, field_view) in view.fields { let child_path = format!("{path}.{child}", child = ChildName(&field_meta.name)); - let field_deserializer = ArrayDeserializer::new( + let field_deserializer = ArrayDeserializer::new_with_utf8_validation( child_path, get_strategy_from_metadata(&field_meta.metadata)?.as_ref(), field_view, + utf8_validation, )?; let field_name = field_meta.name; diff --git a/serde_arrow/src/internal/deserializer.rs b/serde_arrow/src/internal/deserializer.rs index b24449de..26650ea9 100644 --- a/serde_arrow/src/internal/deserializer.rs +++ b/serde_arrow/src/internal/deserializer.rs @@ -5,6 +5,7 @@ use marrow::{datatypes::Field, view::View}; use crate::internal::{ deserialization::{ array_deserializer::ArrayDeserializer, struct_deserializer::StructDeserializer, + Utf8Validation, }, error::{fail, Error, Result}, schema::get_strategy_from_metadata, @@ -90,6 +91,14 @@ pub struct Deserializer<'de> { impl<'de> Deserializer<'de> { pub(crate) fn new(fields: &[Field], views: Vec>) -> Result { + Self::new_with_utf8_validation(fields, views, Utf8Validation::CHECKED) + } + + pub(crate) fn new_with_utf8_validation( + fields: &[Field], + views: Vec>, + utf8_validation: Utf8Validation, + ) -> Result { let len = match views.first() { Some(view) => view.len()?, None => 0, @@ -105,10 +114,11 @@ impl<'de> Deserializer<'de> { ); } let strategy = get_strategy_from_metadata(&field.metadata)?; - let deserializer = ArrayDeserializer::new( + let deserializer = ArrayDeserializer::new_with_utf8_validation( format!("$.{child}", child = ChildName(&field.name)), strategy.as_ref(), view, + utf8_validation, )?; deserializers.push((field.name.clone(), deserializer)); } diff --git a/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs b/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs index 99495df1..76c4b32a 100644 --- a/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs +++ b/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs @@ -1,7 +1,8 @@ use super::utils::Test; use crate::{ internal::deserialization::{ - array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, + array_deserializer::ArrayDeserializer, + random_access_deserializer::RandomAccessDeserializer, Utf8Validation, }, schema::TracingOptions, utils::{Item, Items}, @@ -134,8 +135,13 @@ mod construction { values: Box::new(to_array(DataType::Utf8, false, ["foo", "bar"])), }); - let deserializer = - ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); + let deserializer = ArrayDeserializer::new_with_utf8_validation( + String::from("$"), + None, + array.as_view(), + Utf8Validation::CHECKED, + ) + .unwrap(); assert_eq!( String::deserialize(deserializer.at(0)).unwrap(), @@ -158,8 +164,13 @@ mod construction { values: Box::new(to_array(DataType::Utf8, true, [Some("foo"), Some("bar")])), }); - let deserializer = - ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); + let deserializer = ArrayDeserializer::new_with_utf8_validation( + String::from("$"), + None, + array.as_view(), + Utf8Validation::CHECKED, + ) + .unwrap(); assert_eq!( String::deserialize(deserializer.at(0)).unwrap(), @@ -182,8 +193,13 @@ mod construction { values: Box::new(to_array(DataType::Utf8, true, [None::<&str>, None])), }); - let deserializer = - ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); + let deserializer = ArrayDeserializer::new_with_utf8_validation( + String::from("$"), + None, + array.as_view(), + Utf8Validation::CHECKED, + ) + .unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(), @@ -210,8 +226,13 @@ mod construction { )), }); - let deserializer = - ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); + let deserializer = ArrayDeserializer::new_with_utf8_validation( + String::from("$"), + None, + array.as_view(), + Utf8Validation::CHECKED, + ) + .unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(), @@ -234,8 +255,13 @@ mod construction { values: Box::new(to_array(DataType::Utf8, false, ["foo", "bar"])), }); - let deserializer = - ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); + let deserializer = ArrayDeserializer::new_with_utf8_validation( + String::from("$"), + None, + array.as_view(), + Utf8Validation::CHECKED, + ) + .unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(), From aab3476031b58daa410923afe361e0d7601f1144 Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Fri, 2 Oct 2026 22:54:06 +0200 Subject: [PATCH 6/8] Use field cursor for ordered struct deserialization --- .../deserialization/struct_deserializer.rs | 26 ++++++++++++++----- serde_arrow_bench/benches/groups/complex.rs | 5 ++-- .../benches/groups/primitives.rs | 5 ++-- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/serde_arrow/src/internal/deserialization/struct_deserializer.rs b/serde_arrow/src/internal/deserialization/struct_deserializer.rs index 271a5078..7ed1fd97 100644 --- a/serde_arrow/src/internal/deserialization/struct_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/struct_deserializer.rs @@ -133,7 +133,7 @@ impl<'de> RandomAccessDeserializer<'de> for StructDeserializer<'de> { ) -> Result { if self.fields_match(fields) { visitor - .visit_seq(StructItemDeserializer::new(self, idx)) + .visit_seq(StructSeqDeserializer::new(self, idx)) .ctx(self) } else { visitor @@ -149,7 +149,7 @@ impl<'de> RandomAccessDeserializer<'de> for StructDeserializer<'de> { idx: usize, ) -> Result { visitor - .visit_seq(StructItemDeserializer::new(self, idx)) + .visit_seq(StructSeqDeserializer::new(self, idx)) .ctx(self) } @@ -161,7 +161,7 @@ impl<'de> RandomAccessDeserializer<'de> for StructDeserializer<'de> { idx: usize, ) -> Result { visitor - .visit_seq(StructItemDeserializer::new(self, idx)) + .visit_seq(StructSeqDeserializer::new(self, idx)) .ctx(self) } } @@ -209,16 +209,30 @@ impl<'de> MapAccess<'de> for StructItemDeserializer<'_, 'de> { } } -impl<'de> SeqAccess<'de> for StructItemDeserializer<'_, 'de> { +struct StructSeqDeserializer<'a, 'de> { + fields: &'a [(String, ArrayDeserializer<'de>)], + item: usize, +} + +impl<'a, 'de> StructSeqDeserializer<'a, 'de> { + fn new(deserializer: &'a StructDeserializer<'de>, item: usize) -> Self { + Self { + fields: &deserializer.fields, + item, + } + } +} + +impl<'de> SeqAccess<'de> for StructSeqDeserializer<'_, 'de> { type Error = Error; fn next_element_seed>(&mut self, seed: T) -> Result> { - let Some((_, field_deserializer)) = self.deserializer.fields.get(self.field) else { + let Some(((_, field_deserializer), rest)) = self.fields.split_first() else { return Ok(None); }; let res = seed.deserialize(field_deserializer.at(self.item))?; - self.field += 1; + self.fields = rest; Ok(Some(res)) } diff --git a/serde_arrow_bench/benches/groups/complex.rs b/serde_arrow_bench/benches/groups/complex.rs index ea79e615..60a5c5bc 100644 --- a/serde_arrow_bench/benches/groups/complex.rs +++ b/serde_arrow_bench/benches/groups/complex.rs @@ -11,7 +11,7 @@ use arrow_schema::{DataType, Field, Schema}; use rand::{ distributions::{Standard, Uniform}, prelude::Distribution, - Rng, + Rng, SeedableRng, }; use serde::{Deserialize, Serialize}; @@ -95,8 +95,9 @@ pub fn benchmark_serialize(c: &mut criterion::Criterion) { } pub fn benchmark_deserialize(c: &mut criterion::Criterion) { + let mut rng = rand::rngs::StdRng::seed_from_u64(42); let items = (0..1_000) - .map(|_| Item::random(&mut rand::thread_rng())) + .map(|_| Item::random(&mut rng)) .collect::>(); let mut group = super::new_group(c, "complex_1000_deserialize"); diff --git a/serde_arrow_bench/benches/groups/primitives.rs b/serde_arrow_bench/benches/groups/primitives.rs index 13c55b0f..4e28f620 100644 --- a/serde_arrow_bench/benches/groups/primitives.rs +++ b/serde_arrow_bench/benches/groups/primitives.rs @@ -11,7 +11,7 @@ use arrow_schema::Schema; use rand::{ distributions::{Standard, Uniform}, prelude::Distribution, - Rng, + Rng, SeedableRng, }; use serde::{Deserialize, Serialize}; @@ -94,8 +94,9 @@ pub fn benchmark_serialize(c: &mut criterion::Criterion) { } pub fn benchmark_deserialize(c: &mut criterion::Criterion) { + let mut rng = rand::rngs::StdRng::seed_from_u64(42); let items = (0..1_000) - .map(|_| Item::random(&mut rand::thread_rng())) + .map(|_| Item::random(&mut rng)) .collect::>(); let mut group = super::new_group(c, "primitives_1000_deserialize"); From c08184b66c13549598c03ac45eecd612dfd9c374 Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Fri, 2 Oct 2026 23:53:48 +0200 Subject: [PATCH 7/8] Avoid repeated UTF-8 validation for nullable Marrow strings --- .../deserialization/array_deserializer.rs | 18 +- .../deserialization/string_deserializer.rs | 157 ++++++++++++++++++ serde_arrow_bench/benches/groups/mod.rs | 1 + .../benches/groups/nullable_strings.rs | 64 +++++++ .../benches/serde_arrow_bench.rs | 1 + 5 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 serde_arrow_bench/benches/groups/nullable_strings.rs diff --git a/serde_arrow/src/internal/deserialization/array_deserializer.rs b/serde_arrow/src/internal/deserialization/array_deserializer.rs index 3e9f6ea7..429563ac 100644 --- a/serde_arrow/src/internal/deserialization/array_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/array_deserializer.rs @@ -6,7 +6,7 @@ use serde::{ }; use crate::internal::{ - error::{fail, Context, Error, Result}, + error::{fail, try_, Context, ContextSupport, Error, Result}, schema::Strategy, }; @@ -308,6 +308,22 @@ impl<'de> RandomAccessDeserializer<'de> for ArrayDeserializer<'de> { dispatch!(self, Self(this) => this.is_some(idx)) } + fn deserialize_option>(&self, visitor: V, idx: usize) -> Result { + match self { + Self::Utf8(this) => this.deserialize_option(visitor, idx), + Self::LargeUtf8(this) => this.deserialize_option(visitor, idx), + Self::Utf8View(this) => this.deserialize_option(visitor, idx), + _ => try_(|| { + if self.is_some(idx)? { + visitor.visit_some(self.at(idx)) + } else { + visitor.visit_none() + } + }) + .ctx(self), + } + } + fn deserialize_any_some>(&self, visitor: V, idx: usize) -> Result { dispatch!(self, Self(this) => this.deserialize_any_some(visitor, idx)) } diff --git a/serde_arrow/src/internal/deserialization/string_deserializer.rs b/serde_arrow/src/internal/deserialization/string_deserializer.rs index 2751cc1d..bb28a52d 100644 --- a/serde_arrow/src/internal/deserialization/string_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/string_deserializer.rs @@ -75,6 +75,35 @@ where } } + fn deserialize_option>( + &self, + visitor: V, + idx: usize, + ) -> Result { + if self.utf8_validation.is_trusted() { + return try_(|| { + if self.is_some(idx)? { + visitor.visit_some(self.at(idx)) + } else { + visitor.visit_none() + } + }) + .ctx(self); + } + + try_(|| match ViewAccess::::get(&self.view, idx)? { + Some(value) => visitor.visit_some( + ValidatedStringDeserializer { + original: self, + value, + } + .at(idx), + ), + None => visitor.visit_none(), + }) + .ctx(self) + } + fn deserialize_any_some>( &self, visitor: V, @@ -139,3 +168,131 @@ where .ctx(self) } } + +struct ValidatedStringDeserializer<'this, 'a, VV> { + original: &'this StringDeserializer, + value: &'a str, +} + +impl Context for ValidatedStringDeserializer<'_, '_, VV> { + fn annotate(&self, annotations: &mut std::collections::BTreeMap) { + self.original.annotate(annotations); + } +} + +impl<'a, VV> RandomAccessDeserializer<'a> for ValidatedStringDeserializer<'_, 'a, VV> +where + VV: StringDeserializerDataType, +{ + fn is_some(&self, _idx: usize) -> Result { + Ok(true) + } + + fn deserialize_any_some>( + &self, + visitor: V, + idx: usize, + ) -> Result { + self.deserialize_str(visitor, idx) + } + + fn deserialize_str>( + &self, + visitor: V, + _idx: usize, + ) -> Result { + try_(|| visitor.visit_borrowed_str(self.value)).ctx(self) + } + + fn deserialize_string>( + &self, + visitor: V, + _idx: usize, + ) -> Result { + try_(|| visitor.visit_string(self.value.to_owned())).ctx(self) + } + + fn deserialize_bytes>( + &self, + visitor: V, + _idx: usize, + ) -> Result { + try_(|| visitor.visit_bytes(self.value.as_bytes())).ctx(self) + } + + fn deserialize_byte_buf>( + &self, + visitor: V, + _idx: usize, + ) -> Result { + try_(|| visitor.visit_byte_buf(self.value.to_owned().into_bytes())).ctx(self) + } + + fn deserialize_enum>( + &self, + _name: &'static str, + _variants: &'static [&'static str], + visitor: V, + _idx: usize, + ) -> Result { + try_(|| visitor.visit_enum(EnumAccess(self.value))).ctx(self) + } +} + +#[cfg(test)] +mod tests { + use marrow::{ + datatypes::{DataType, Field}, + view::{BitsWithOffset, BytesView, View}, + }; + use serde::Deserialize; + + #[derive(Debug, Deserialize, PartialEq)] + struct Row<'a> { + #[serde(borrow)] + value: Option<&'a str>, + } + + #[test] + fn nullable_marrow_string_borrows_valid_values_and_reports_invalid_utf8() { + let fields = [Field { + name: "value".into(), + data_type: DataType::LargeUtf8, + nullable: true, + metadata: Default::default(), + }]; + let offsets = [0_i64, 5, 5]; + let valid = [0b0000_0001]; + let views = [View::LargeUtf8(BytesView { + offsets: &offsets, + data: "café".as_bytes(), + validity: Some(BitsWithOffset { + offset: 0, + data: &valid, + }), + })]; + let decoded: Vec> = crate::from_marrow(&fields, &views).unwrap(); + assert_eq!( + decoded, + vec![ + Row { + value: Some("café") + }, + Row { value: None } + ] + ); + + let invalid_offsets = [0_i64, 2]; + let invalid = [b'a', 0xff]; + let views = [View::LargeUtf8(BytesView { + offsets: &invalid_offsets, + data: &invalid, + validity: Some(BitsWithOffset { + offset: 0, + data: &valid, + }), + })]; + let error = crate::from_marrow::>>(&fields, &views).unwrap_err(); + assert!(error.to_string().contains("invalid UTF-8 data")); + } +} diff --git a/serde_arrow_bench/benches/groups/mod.rs b/serde_arrow_bench/benches/groups/mod.rs index 91fe2e95..cb1d8e82 100644 --- a/serde_arrow_bench/benches/groups/mod.rs +++ b/serde_arrow_bench/benches/groups/mod.rs @@ -3,6 +3,7 @@ use criterion::measurement::Measurement; pub mod binary; pub mod complex; pub mod json_to_arrow; +pub mod nullable_strings; pub mod primitives; pub mod wide_schema; diff --git a/serde_arrow_bench/benches/groups/nullable_strings.rs b/serde_arrow_bench/benches/groups/nullable_strings.rs new file mode 100644 index 00000000..fd868222 --- /dev/null +++ b/serde_arrow_bench/benches/groups/nullable_strings.rs @@ -0,0 +1,64 @@ +use arrow_array::{Array, ArrayRef, LargeStringArray}; +use criterion::Criterion; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, PartialEq, Deserialize, Serialize)] +struct Item { + value: Option, +} + +pub fn benchmark_deserialize(c: &mut Criterion) { + let items = (0..1_000) + .map(|idx| Item { + value: (idx % 10 != 0).then(|| format!("{idx:04}{}", "é".repeat(64))), + }) + .collect::>(); + let mut group = super::new_group(c, "nullable_strings_1000_deserialize"); + + let arrow_fields = crate::impls::serde_arrow_arrow::trace(&items); + let arrow_arrays = serde_arrow::to_arrow(&arrow_fields, &items).unwrap(); + assert_eq!(deserialize_manual(&arrow_arrays), items); + group.bench_function("arrow_manual", |b| { + b.iter(|| criterion::black_box(deserialize_manual(&arrow_arrays))) + }); + let decoded: Vec = serde_arrow::from_arrow(&arrow_fields, &arrow_arrays).unwrap(); + assert_eq!(decoded, items); + group.bench_function("serde_arrow_arrow", |b| { + b.iter(|| { + let decoded: Vec = serde_arrow::from_arrow(&arrow_fields, &arrow_arrays).unwrap(); + criterion::black_box(decoded) + }) + }); + + let marrow_fields = crate::impls::serde_arrow_marrow::trace(&items); + let marrow_arrays = serde_arrow::to_marrow(&marrow_fields, &items).unwrap(); + let marrow_views = marrow_arrays + .iter() + .map(|array| array.as_view()) + .collect::>(); + let decoded: Vec = serde_arrow::from_marrow(&marrow_fields, &marrow_views).unwrap(); + assert_eq!(decoded, items); + group.bench_function("serde_arrow_marrow", |b| { + b.iter(|| { + let decoded: Vec = + serde_arrow::from_marrow(&marrow_fields, &marrow_views).unwrap(); + criterion::black_box(decoded) + }) + }); + + group.finish(); +} + +fn deserialize_manual(arrays: &[ArrayRef]) -> Vec { + let values = arrays[0] + .as_any() + .downcast_ref::() + .unwrap(); + (0..values.len()) + .map(|idx| Item { + value: (!values.is_null(idx)).then(|| values.value(idx).to_owned()), + }) + .collect() +} + +criterion::criterion_group!(benchmark, benchmark_deserialize); diff --git a/serde_arrow_bench/benches/serde_arrow_bench.rs b/serde_arrow_bench/benches/serde_arrow_bench.rs index 92d2f440..7523be51 100644 --- a/serde_arrow_bench/benches/serde_arrow_bench.rs +++ b/serde_arrow_bench/benches/serde_arrow_bench.rs @@ -4,6 +4,7 @@ pub mod impls; criterion::criterion_main!( groups::binary::benchmark, groups::complex::benchmark, + groups::nullable_strings::benchmark, groups::primitives::benchmark, groups::json_to_arrow::benchmark, groups::wide_schema::benchmark, From 7110c6edabcdcc2ac20e972a1fe60c819cefa5a7 Mon Sep 17 00:00:00 2001 From: Christopher Prohm Date: Sat, 3 Oct 2026 17:14:49 +0200 Subject: [PATCH 8/8] roll back optimizations requiring unsafe --- serde_arrow/src/arrow_impl.rs | 139 +----------------- .../deserialization/array_deserializer.rs | 84 ++++------- .../dictionary_deserializer.rs | 12 +- .../deserialization/enum_deserializer.rs | 6 +- .../fixed_size_list_deserializer.rs | 11 +- .../deserialization/list_deserializer.rs | 11 +- .../deserialization/map_deserializer.rs | 10 +- .../src/internal/deserialization/mod.rs | 20 --- .../deserialization/string_deserializer.rs | 63 ++------ .../deserialization/struct_deserializer.rs | 11 +- serde_arrow/src/internal/deserializer.rs | 12 +- .../test_with_arrow/impls/arrow_dictionary.rs | 48 ++---- 12 files changed, 69 insertions(+), 358 deletions(-) diff --git a/serde_arrow/src/arrow_impl.rs b/serde_arrow/src/arrow_impl.rs index 3827d8d5..21e5627f 100644 --- a/serde_arrow/src/arrow_impl.rs +++ b/serde_arrow/src/arrow_impl.rs @@ -15,7 +15,6 @@ use crate::{ }, internal::{ array_builder::ArrayBuilder, - deserialization::Utf8Validation, deserializer::Deserializer, error::{fail, Error, ErrorKind, Result}, schema::extensions::{Bool8Field, FixedShapeTensorField, VariableShapeTensorField}, @@ -282,11 +281,7 @@ impl<'de> Deserializer<'de> { views.push(View::try_from(array.as_ref())?); } - // SAFETY: each view was converted from an Arrow array. Arrow's unsafe - // Array contract requires UTF-8 string values to be valid, including - // values held by nested arrays and dictionaries. - let utf8_validation = unsafe { Utf8Validation::trusted_arrow() }; - Deserializer::new_with_utf8_validation(&fields, views, utf8_validation) + Deserializer::new(&fields, views) } /// Construct a new deserializer from a record batch (*requires one of the @@ -451,135 +446,3 @@ macro_rules! impl_try_from_ext_type { impl_try_from_ext_type!(Bool8Field); impl_try_from_ext_type!(FixedShapeTensorField); impl_try_from_ext_type!(VariableShapeTensorField); - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use serde::{Deserialize, Serialize}; - - use crate::{ - _impl::arrow::datatypes::{DataType, Field as ArrowField, FieldRef}, - schema::{SchemaLike, TracingOptions}, - }; - - use super::{from_arrow, to_arrow}; - - #[derive(Debug, Deserialize, Serialize, PartialEq)] - struct OptionalString { - value: Option, - } - - #[derive(Debug, Deserialize, PartialEq)] - struct BorrowedString<'a> { - #[serde(borrow)] - value: Option<&'a str>, - } - - #[test] - fn trusted_arrow_string_types() { - let expected = vec![ - OptionalString { - value: Some("café".into()), - }, - OptionalString { value: None }, - OptionalString { - value: Some("a string longer than twelve bytes".into()), - }, - ]; - - for data_type in [DataType::Utf8, DataType::LargeUtf8, DataType::Utf8View] { - let fields = vec![Arc::new(ArrowField::new("value", data_type, true))]; - let arrays = to_arrow(&fields, &expected).unwrap(); - let actual: Vec = from_arrow(&fields, &arrays).unwrap(); - assert_eq!(actual, expected); - let borrowed: Vec> = from_arrow(&fields, &arrays).unwrap(); - assert_eq!(borrowed[0].value, Some("café")); - assert_eq!(borrowed[1].value, None); - } - } - - #[derive(Debug, Deserialize, Serialize, PartialEq)] - struct NestedStrings { - child: Child, - strings: Vec, - } - - #[derive(Debug, Deserialize, Serialize, PartialEq)] - struct Child { - label: String, - } - - #[test] - fn trusted_arrow_nested_strings() { - let items = vec![NestedStrings { - child: Child { - label: "éclair".into(), - }, - strings: vec!["one".into(), "zürich".into()], - }]; - let fields = - Vec::::from_type::(TracingOptions::default()).unwrap(); - let arrays = to_arrow(&fields, &items).unwrap(); - let actual: Vec = from_arrow(&fields, &arrays).unwrap(); - assert_eq!(actual, items); - } - - #[derive(Debug, Deserialize, PartialEq)] - struct RequiredString { - value: String, - } - - #[test] - fn trusted_arrow_dictionary_strings() { - use marrow::array::{Array as MarrowArray, BytesArray, DictionaryArray, PrimitiveArray}; - - let dictionary = MarrowArray::Dictionary(DictionaryArray { - keys: Box::new(MarrowArray::Int8(PrimitiveArray { - values: vec![1, 0], - validity: None, - })), - values: Box::new(MarrowArray::Utf8(BytesArray { - offsets: vec![0, 5, 12], - data: "cafézürich".as_bytes().to_vec(), - validity: None, - })), - }); - let array = crate::_impl::arrow::array::ArrayRef::try_from(dictionary).unwrap(); - let fields = vec![Arc::new(ArrowField::new( - "value", - array.data_type().clone(), - false, - ))]; - - let actual: Vec = from_arrow(&fields, &[array]).unwrap(); - assert_eq!( - actual, - vec![ - RequiredString { - value: "zürich".into() - }, - RequiredString { - value: "café".into() - }, - ] - ); - } - - #[test] - fn marrow_strings_still_validate_utf8() { - let fields = vec![marrow::datatypes::Field { - name: "value".into(), - data_type: marrow::datatypes::DataType::Utf8, - ..marrow::datatypes::Field::default() - }]; - let views = [marrow::view::View::Utf8(marrow::view::BytesView { - offsets: &[0, 1], - data: &[0xff], - validity: None, - })]; - - let result = crate::from_marrow::>(&fields, &views); - assert!(result.is_err()); - } -} diff --git a/serde_arrow/src/internal/deserialization/array_deserializer.rs b/serde_arrow/src/internal/deserialization/array_deserializer.rs index 429563ac..65b52eb1 100644 --- a/serde_arrow/src/internal/deserialization/array_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/array_deserializer.rs @@ -30,7 +30,6 @@ use super::{ struct_deserializer::StructDeserializer, time_deserializer::TimeDeserializer, timestamp_deserializer::TimestampDeserializer, - Utf8Validation, }; pub enum ArrayDeserializer<'a> { @@ -87,12 +86,7 @@ pub enum ArrayDeserializer<'a> { impl<'a> ArrayDeserializer<'a> { // TODO: decide whether to keep strategy parameter - pub fn new_with_utf8_validation( - path: String, - _strategy: Option<&Strategy>, - array: View<'a>, - utf8_validation: Utf8Validation, - ) -> Result { + pub fn new(path: String, _strategy: Option<&Strategy>, array: View<'a>) -> Result { use {ArrayDeserializer as D, View as V}; match array { View::Null(_) => Ok(Self::Null(NullDeserializer::new(path))), @@ -115,97 +109,71 @@ impl<'a> ArrayDeserializer<'a> { V::Time64(view) => Ok(D::Time64(TimeDeserializer::new(path, view))), V::Timestamp(view) => Ok(Self::Timestamp(TimestampDeserializer::new(path, view)?)), V::Duration(view) => Ok(D::Duration(DurationDeserializer::new(path, view))), - V::Utf8(view) => Ok(D::Utf8(StringDeserializer::new( - path, - view, - utf8_validation, - ))), - V::LargeUtf8(view) => Ok(D::LargeUtf8(StringDeserializer::new( - path, - view, - utf8_validation, - ))), - V::Utf8View(view) => Ok(D::Utf8View(StringDeserializer::new( - path, - view, - utf8_validation, - ))), + V::Utf8(view) => Ok(D::Utf8(StringDeserializer::new(path, view))), + V::LargeUtf8(view) => Ok(D::LargeUtf8(StringDeserializer::new(path, view))), + V::Utf8View(view) => Ok(D::Utf8View(StringDeserializer::new(path, view))), V::Binary(view) => Ok(D::Binary(BinaryDeserializer::new(path, view))), V::LargeBinary(view) => Ok(D::LargeBinary(BinaryDeserializer::new(path, view))), V::BinaryView(view) => Ok(D::BinaryView(BinaryDeserializer::new(path, view))), V::FixedSizeBinary(view) => Ok(D::FixedSizeBinary(FixedSizeBinaryDeserializer::new( path, view, )?)), - V::List(view) => Ok(D::List(ListDeserializer::new(path, view, utf8_validation)?)), - V::LargeList(view) => Ok(D::LargeList(ListDeserializer::new( - path, - view, - utf8_validation, - )?)), + V::List(view) => Ok(D::List(ListDeserializer::new(path, view)?)), + V::LargeList(view) => Ok(D::LargeList(ListDeserializer::new(path, view)?)), V::FixedSizeList(view) => Ok(D::FixedSizeList(FixedSizeListDeserializer::new( - path, - view, - utf8_validation, - )?)), - V::Struct(view) => Ok(D::Struct(StructDeserializer::new( - path, - view, - utf8_validation, - )?)), - V::Map(view) => Ok(D::Map(MapDeserializer::new(path, view, utf8_validation)?)), - View::Union(view) => Ok(Self::Enum(EnumDeserializer::new( - path, - view, - utf8_validation, + path, view, )?)), + V::Struct(view) => Ok(D::Struct(StructDeserializer::new(path, view)?)), + V::Map(view) => Ok(D::Map(MapDeserializer::new(path, view)?)), + View::Union(view) => Ok(Self::Enum(EnumDeserializer::new(path, view)?)), V::Dictionary(view) => match (*view.keys, *view.values) { (V::Int8(keys), V::Utf8(values)) => Ok(D::DictionaryI8I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int16(keys), V::Utf8(values)) => Ok(D::DictionaryI16I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int32(keys), V::Utf8(values)) => Ok(D::DictionaryI32I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int64(keys), V::Utf8(values)) => Ok(D::DictionaryI64I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt8(keys), V::Utf8(values)) => Ok(Self::DictionaryU8I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt16(keys), V::Utf8(values)) => Ok(D::DictionaryU16I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt32(keys), V::Utf8(values)) => Ok(D::DictionaryU32I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt64(keys), V::Utf8(values)) => Ok(D::DictionaryU64I32( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int8(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI8I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int16(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI16I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int32(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI32I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::Int64(keys), V::LargeUtf8(values)) => Ok(D::DictionaryI64I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt8(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU8I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt16(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU16I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt32(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU32I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), (V::UInt64(keys), V::LargeUtf8(values)) => Ok(D::DictionaryU64I64( - DictionaryDeserializer::new(path, keys, values, utf8_validation)?, + DictionaryDeserializer::new(path, keys, values)?, )), _ => fail!("unsupported dictionary array type"), }, diff --git a/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs b/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs index e0e7971b..4787b289 100644 --- a/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/dictionary_deserializer.rs @@ -9,34 +9,26 @@ use crate::internal::{ use super::{ enums_as_string_impl::EnumAccess, integer_deserializer::Integer, random_access_deserializer::RandomAccessDeserializer, string_deserializer::get_utf8, - Utf8Validation, }; pub struct DictionaryDeserializer<'a, K: Integer, V: Offset> { path: String, keys: PrimitiveView<'a, K>, values: BytesView<'a, V>, - utf8_validation: Utf8Validation, } impl<'a, K: Integer, V: Offset> DictionaryDeserializer<'a, K, V> { - pub fn new( - path: String, - keys: PrimitiveView<'a, K>, - values: BytesView<'a, V>, - utf8_validation: Utf8Validation, - ) -> Result { + pub fn new(path: String, keys: PrimitiveView<'a, K>, values: BytesView<'a, V>) -> Result { Ok(Self { path, keys: keys.clone(), values: values.clone(), - utf8_validation, }) } pub fn get_str(&self, idx: usize) -> Result<&str> { let key: usize = self.keys.get_required(idx)?.into_i64()?.try_into()?; - let value: &str = get_utf8(&self.values, key, self.utf8_validation)?; + let value: &str = get_utf8(&self.values, key)?; Ok(value) } } diff --git a/serde_arrow/src/internal/deserialization/enum_deserializer.rs b/serde_arrow/src/internal/deserialization/enum_deserializer.rs index 6e4daec6..e35f0ea0 100644 --- a/serde_arrow/src/internal/deserialization/enum_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/enum_deserializer.rs @@ -10,7 +10,6 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::{PositionedDeserializer, RandomAccessDeserializer}, - Utf8Validation, }; pub struct EnumDeserializer<'a> { @@ -21,7 +20,7 @@ pub struct EnumDeserializer<'a> { } impl<'a> EnumDeserializer<'a> { - pub fn new(path: String, view: UnionView<'a>, utf8_validation: Utf8Validation) -> Result { + pub fn new(path: String, view: UnionView<'a>) -> Result { let Some(offsets) = view.offsets else { fail!("only dense unions are supported"); }; @@ -37,11 +36,10 @@ impl<'a> EnumDeserializer<'a> { fail!("only unions with consecutive type ids are currently supported"); } let child_path = format!("{path}.{child}", child = ChildName(&field_meta.name)); - let field_deserializer = ArrayDeserializer::new_with_utf8_validation( + let field_deserializer = ArrayDeserializer::new( child_path, get_strategy_from_metadata(&field_meta.metadata)?.as_ref(), field_view, - utf8_validation, )?; variants.push((field_meta.name, field_deserializer)) } diff --git a/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs b/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs index c1f06b93..6d870f8c 100644 --- a/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/fixed_size_list_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, list_deserializer::ListItemDeserializer, - random_access_deserializer::RandomAccessDeserializer, utils::bitset_is_set, Utf8Validation, + random_access_deserializer::RandomAccessDeserializer, utils::bitset_is_set, }; pub struct FixedSizeListDeserializer<'a> { @@ -21,17 +21,12 @@ pub struct FixedSizeListDeserializer<'a> { } impl<'a> FixedSizeListDeserializer<'a> { - pub fn new( - path: String, - view: FixedSizeListView<'a>, - utf8_validation: Utf8Validation, - ) -> Result { + pub fn new(path: String, view: FixedSizeListView<'a>) -> Result { let child_path = format!("{path}.{child}", child = ChildName(&view.meta.name)); - let item = ArrayDeserializer::new_with_utf8_validation( + let item = ArrayDeserializer::new( child_path, get_strategy_from_metadata(&view.meta.metadata)?.as_ref(), *view.elements, - utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/list_deserializer.rs b/serde_arrow/src/internal/deserialization/list_deserializer.rs index 72f687f7..c0e2ce89 100644 --- a/serde_arrow/src/internal/deserialization/list_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/list_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, Utf8Validation, + utils::bitset_is_set, }; pub struct ListDeserializer<'a, O: Offset> { @@ -20,17 +20,12 @@ pub struct ListDeserializer<'a, O: Offset> { } impl<'de, O: Offset> ListDeserializer<'de, O> { - pub fn new( - path: String, - view: ListView<'de, O>, - utf8_validation: Utf8Validation, - ) -> Result { + pub fn new(path: String, view: ListView<'de, O>) -> Result { let child_path = format!("{path}.{child}", child = ChildName(&view.meta.name)); - let item = ArrayDeserializer::new_with_utf8_validation( + let item = ArrayDeserializer::new( child_path, get_strategy_from_metadata(&view.meta.metadata)?.as_ref(), *view.elements, - utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/map_deserializer.rs b/serde_arrow/src/internal/deserialization/map_deserializer.rs index 63cfbc88..044d54b2 100644 --- a/serde_arrow/src/internal/deserialization/map_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/map_deserializer.rs @@ -9,7 +9,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, Utf8Validation, + utils::bitset_is_set, }; pub struct MapDeserializer<'a> { @@ -21,17 +21,16 @@ pub struct MapDeserializer<'a> { } impl<'a> MapDeserializer<'a> { - pub fn new(path: String, view: MapView<'a>, utf8_validation: Utf8Validation) -> Result { + pub fn new(path: String, view: MapView<'a>) -> Result { let keys_path = format!( "{path}.{entries}.{keys}", entries = ChildName(&view.meta.entries_name), keys = ChildName(&view.meta.keys.name), ); - let keys = ArrayDeserializer::new_with_utf8_validation( + let keys = ArrayDeserializer::new( keys_path, get_strategy_from_metadata(&view.meta.keys.metadata)?.as_ref(), *view.keys, - utf8_validation, )?; let values_path = format!( @@ -39,11 +38,10 @@ impl<'a> MapDeserializer<'a> { entries = ChildName(&view.meta.entries_name), values = ChildName(&view.meta.values.name), ); - let values = ArrayDeserializer::new_with_utf8_validation( + let values = ArrayDeserializer::new( values_path, get_strategy_from_metadata(&view.meta.values.metadata)?.as_ref(), *view.values, - utf8_validation, )?; Ok(Self { diff --git a/serde_arrow/src/internal/deserialization/mod.rs b/serde_arrow/src/internal/deserialization/mod.rs index bc30d4e2..c63e2ab0 100644 --- a/serde_arrow/src/internal/deserialization/mod.rs +++ b/serde_arrow/src/internal/deserialization/mod.rs @@ -21,23 +21,3 @@ pub mod struct_deserializer; pub mod time_deserializer; pub mod timestamp_deserializer; pub mod utils; - -#[derive(Clone, Copy)] -pub(crate) struct Utf8Validation { - trusted: bool, -} - -impl Utf8Validation { - pub(crate) const CHECKED: Self = Self { trusted: false }; - - /// # Safety - /// Every string view used with this marker must originate from a valid Arrow - /// string array, including nested and dictionary values. - pub(crate) unsafe fn trusted_arrow() -> Self { - Self { trusted: true } - } - - pub(crate) fn is_trusted(self) -> bool { - self.trusted - } -} diff --git a/serde_arrow/src/internal/deserialization/string_deserializer.rs b/serde_arrow/src/internal/deserialization/string_deserializer.rs index bb28a52d..32e78196 100644 --- a/serde_arrow/src/internal/deserialization/string_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/string_deserializer.rs @@ -7,7 +7,6 @@ use crate::internal::{ use super::{ enums_as_string_impl::EnumAccess, random_access_deserializer::RandomAccessDeserializer, - Utf8Validation, }; pub trait StringDeserializerDataType { @@ -29,31 +28,19 @@ impl StringDeserializerDataType for BytesViewView<'_> { pub struct StringDeserializer { pub path: String, pub view: V, - utf8_validation: Utf8Validation, } impl StringDeserializer { - pub fn new(path: String, view: V, utf8_validation: Utf8Validation) -> Self { - Self { - path, - view, - utf8_validation, - } + pub fn new(path: String, view: V) -> Self { + Self { path, view } } } -pub(super) fn get_utf8<'a, V>(view: &V, idx: usize, validation: Utf8Validation) -> Result<&'a str> +pub(super) fn get_utf8<'a, V>(view: &V, idx: usize) -> Result<&'a str> where - V: ViewAccess<'a, str> + ViewAccess<'a, [u8]> + 'a, + V: ViewAccess<'a, str> + 'a, { - if validation.is_trusted() { - let bytes = ViewAccess::<[u8]>::get_required(view, idx)?; - // SAFETY: the trusted marker requires all string views to come from - // valid Arrow arrays. Their unsafe Array contract guarantees UTF-8. - Ok(unsafe { std::str::from_utf8_unchecked(bytes) }) - } else { - ViewAccess::::get_required(view, idx) - } + ViewAccess::::get_required(view, idx) } impl Context for StringDeserializer { @@ -65,14 +52,10 @@ impl Context for StringDeserializer { impl<'a, VV> RandomAccessDeserializer<'a> for StringDeserializer where - VV: ViewAccess<'a, str> + ViewAccess<'a, [u8]> + StringDeserializerDataType + 'a, + VV: ViewAccess<'a, str> + StringDeserializerDataType + 'a, { fn is_some(&self, idx: usize) -> Result { - if self.utf8_validation.is_trusted() { - ViewAccess::<[u8]>::is_some(&self.view, idx) - } else { - ViewAccess::::is_some(&self.view, idx) - } + self.view.is_some(idx) } fn deserialize_option>( @@ -80,17 +63,6 @@ where visitor: V, idx: usize, ) -> Result { - if self.utf8_validation.is_trusted() { - return try_(|| { - if self.is_some(idx)? { - visitor.visit_some(self.at(idx)) - } else { - visitor.visit_none() - } - }) - .ctx(self); - } - try_(|| match ViewAccess::::get(&self.view, idx)? { Some(value) => visitor.visit_some( ValidatedStringDeserializer { @@ -117,8 +89,7 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_borrowed_str(get_utf8(&self.view, idx, self.utf8_validation)?)) - .ctx(self) + try_(|| visitor.visit_borrowed_str(get_utf8(&self.view, idx)?)).ctx(self) } fn deserialize_string>( @@ -126,8 +97,7 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_string(get_utf8(&self.view, idx, self.utf8_validation)?.to_owned())) - .ctx(self) + try_(|| visitor.visit_string(get_utf8(&self.view, idx)?.to_owned())).ctx(self) } fn deserialize_bytes>( @@ -135,8 +105,7 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| visitor.visit_bytes(get_utf8(&self.view, idx, self.utf8_validation)?.as_bytes())) - .ctx(self) + try_(|| visitor.visit_bytes(get_utf8(&self.view, idx)?.as_bytes())).ctx(self) } fn deserialize_byte_buf>( @@ -144,14 +113,8 @@ where visitor: V, idx: usize, ) -> Result { - try_(|| { - visitor.visit_byte_buf( - get_utf8(&self.view, idx, self.utf8_validation)? - .to_owned() - .into_bytes(), - ) - }) - .ctx(self) + try_(|| visitor.visit_byte_buf(get_utf8(&self.view, idx)?.to_owned().into_bytes())) + .ctx(self) } fn deserialize_enum>( @@ -162,7 +125,7 @@ where idx: usize, ) -> Result { try_(|| { - let variant = get_utf8(&self.view, idx, self.utf8_validation)?; + let variant = get_utf8(&self.view, idx)?; visitor.visit_enum(EnumAccess(variant)) }) .ctx(self) diff --git a/serde_arrow/src/internal/deserialization/struct_deserializer.rs b/serde_arrow/src/internal/deserialization/struct_deserializer.rs index 7ed1fd97..3d703066 100644 --- a/serde_arrow/src/internal/deserialization/struct_deserializer.rs +++ b/serde_arrow/src/internal/deserialization/struct_deserializer.rs @@ -11,7 +11,7 @@ use crate::internal::{ use super::{ array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, - utils::bitset_is_set, Utf8Validation, + utils::bitset_is_set, }; pub struct StructDeserializer<'a> { @@ -23,19 +23,14 @@ pub struct StructDeserializer<'a> { } impl<'a> StructDeserializer<'a> { - pub fn new( - path: String, - view: StructView<'a>, - utf8_validation: Utf8Validation, - ) -> Result { + pub fn new(path: String, view: StructView<'a>) -> Result { let mut fields = Vec::with_capacity(view.fields.len()); for (field_meta, field_view) in view.fields { let child_path = format!("{path}.{child}", child = ChildName(&field_meta.name)); - let field_deserializer = ArrayDeserializer::new_with_utf8_validation( + let field_deserializer = ArrayDeserializer::new( child_path, get_strategy_from_metadata(&field_meta.metadata)?.as_ref(), field_view, - utf8_validation, )?; let field_name = field_meta.name; diff --git a/serde_arrow/src/internal/deserializer.rs b/serde_arrow/src/internal/deserializer.rs index 26650ea9..b24449de 100644 --- a/serde_arrow/src/internal/deserializer.rs +++ b/serde_arrow/src/internal/deserializer.rs @@ -5,7 +5,6 @@ use marrow::{datatypes::Field, view::View}; use crate::internal::{ deserialization::{ array_deserializer::ArrayDeserializer, struct_deserializer::StructDeserializer, - Utf8Validation, }, error::{fail, Error, Result}, schema::get_strategy_from_metadata, @@ -91,14 +90,6 @@ pub struct Deserializer<'de> { impl<'de> Deserializer<'de> { pub(crate) fn new(fields: &[Field], views: Vec>) -> Result { - Self::new_with_utf8_validation(fields, views, Utf8Validation::CHECKED) - } - - pub(crate) fn new_with_utf8_validation( - fields: &[Field], - views: Vec>, - utf8_validation: Utf8Validation, - ) -> Result { let len = match views.first() { Some(view) => view.len()?, None => 0, @@ -114,11 +105,10 @@ impl<'de> Deserializer<'de> { ); } let strategy = get_strategy_from_metadata(&field.metadata)?; - let deserializer = ArrayDeserializer::new_with_utf8_validation( + let deserializer = ArrayDeserializer::new( format!("$.{child}", child = ChildName(&field.name)), strategy.as_ref(), view, - utf8_validation, )?; deserializers.push((field.name.clone(), deserializer)); } diff --git a/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs b/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs index 76c4b32a..99495df1 100644 --- a/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs +++ b/serde_arrow/src/test_with_arrow/impls/arrow_dictionary.rs @@ -1,8 +1,7 @@ use super::utils::Test; use crate::{ internal::deserialization::{ - array_deserializer::ArrayDeserializer, - random_access_deserializer::RandomAccessDeserializer, Utf8Validation, + array_deserializer::ArrayDeserializer, random_access_deserializer::RandomAccessDeserializer, }, schema::TracingOptions, utils::{Item, Items}, @@ -135,13 +134,8 @@ mod construction { values: Box::new(to_array(DataType::Utf8, false, ["foo", "bar"])), }); - let deserializer = ArrayDeserializer::new_with_utf8_validation( - String::from("$"), - None, - array.as_view(), - Utf8Validation::CHECKED, - ) - .unwrap(); + let deserializer = + ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); assert_eq!( String::deserialize(deserializer.at(0)).unwrap(), @@ -164,13 +158,8 @@ mod construction { values: Box::new(to_array(DataType::Utf8, true, [Some("foo"), Some("bar")])), }); - let deserializer = ArrayDeserializer::new_with_utf8_validation( - String::from("$"), - None, - array.as_view(), - Utf8Validation::CHECKED, - ) - .unwrap(); + let deserializer = + ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); assert_eq!( String::deserialize(deserializer.at(0)).unwrap(), @@ -193,13 +182,8 @@ mod construction { values: Box::new(to_array(DataType::Utf8, true, [None::<&str>, None])), }); - let deserializer = ArrayDeserializer::new_with_utf8_validation( - String::from("$"), - None, - array.as_view(), - Utf8Validation::CHECKED, - ) - .unwrap(); + let deserializer = + ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(), @@ -226,13 +210,8 @@ mod construction { )), }); - let deserializer = ArrayDeserializer::new_with_utf8_validation( - String::from("$"), - None, - array.as_view(), - Utf8Validation::CHECKED, - ) - .unwrap(); + let deserializer = + ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(), @@ -255,13 +234,8 @@ mod construction { values: Box::new(to_array(DataType::Utf8, false, ["foo", "bar"])), }); - let deserializer = ArrayDeserializer::new_with_utf8_validation( - String::from("$"), - None, - array.as_view(), - Utf8Validation::CHECKED, - ) - .unwrap(); + let deserializer = + ArrayDeserializer::new(String::from("$"), None, array.as_view()).unwrap(); assert_eq!( Option::::deserialize(deserializer.at(0)).unwrap(),