From 3289aee0ea71d1e4a0a55b2956a49476caa27c35 Mon Sep 17 00:00:00 2001 From: Gwo Tzu-Hsing Date: Sat, 23 Nov 2024 15:41:45 +0800 Subject: [PATCH] refactor: remove needless unsafe `Send` `Sync` implementations --- src/record/runtime/array.rs | 195 +++++++++++++++++++------------ src/record/runtime/column.rs | 21 ++-- src/record/runtime/record.rs | 5 +- src/record/runtime/record_ref.rs | 21 ++-- 4 files changed, 139 insertions(+), 103 deletions(-) diff --git a/src/record/runtime/array.rs b/src/record/runtime/array.rs index a0f193e..9cddeeb 100644 --- a/src/record/runtime/array.rs +++ b/src/record/runtime/array.rs @@ -33,7 +33,7 @@ impl ArrowArrays for DynRecordImmutableArrays { type Builder = DynRecordBuilder; fn builder(schema: &Arc, capacity: usize) -> Self::Builder { - let mut builders: Vec> = vec![]; + let mut builders: Vec> = vec![]; let mut datatypes = vec![]; for field in schema.fields().iter().skip(2) { let datatype = Datatype::from(field.data_type()); @@ -120,7 +120,7 @@ impl ArrowArrays for DynRecordImmutableArrays { if projection_mask.leaf_included(idx + 2) && !col.is_nullable { let datatype = col.datatype; let name = col.name.to_string(); - let value: Arc = match datatype { + let value: Arc = match datatype { Datatype::UInt8 => Arc::new(Self::primitive_value::(col, offset)), Datatype::UInt16 => Arc::new(Self::primitive_value::(col, offset)), Datatype::UInt32 => Arc::new(Self::primitive_value::(col, offset)), @@ -184,7 +184,7 @@ impl DynRecordImmutableArrays { } pub struct DynRecordBuilder { - builders: Vec>, + builders: Vec>, datatypes: Vec, _null: BooleanBufferBuilder, _ts: UInt32Builder, @@ -220,7 +220,9 @@ impl Builder for DynRecordBuilder { let datatype = col.datatype; match datatype { Datatype::UInt8 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -228,7 +230,9 @@ impl Builder for DynRecordBuilder { } } Datatype::UInt16 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -236,7 +240,9 @@ impl Builder for DynRecordBuilder { } } Datatype::UInt32 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -244,7 +250,9 @@ impl Builder for DynRecordBuilder { } } Datatype::UInt64 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -252,7 +260,9 @@ impl Builder for DynRecordBuilder { } } Datatype::Int8 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -260,7 +270,9 @@ impl Builder for DynRecordBuilder { } } Datatype::Int16 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -268,7 +280,9 @@ impl Builder for DynRecordBuilder { } } Datatype::Int32 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -276,7 +290,9 @@ impl Builder for DynRecordBuilder { } } Datatype::Int64 => { - let bd = Self::as_builder_mut::>(builder); + let bd = Self::as_builder_mut::>( + builder.as_mut(), + ); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -284,7 +300,7 @@ impl Builder for DynRecordBuilder { } } Datatype::String => { - let bd = Self::as_builder_mut::(builder); + let bd = Self::as_builder_mut::(builder.as_mut()); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { @@ -293,7 +309,7 @@ impl Builder for DynRecordBuilder { } } Datatype::Boolean => { - let bd = Self::as_builder_mut::(builder); + let bd = Self::as_builder_mut::(builder.as_mut()); let value = col.value.as_ref().downcast_ref::>().unwrap(); match value { Some(value) => bd.append_value(*value), @@ -301,7 +317,8 @@ impl Builder for DynRecordBuilder { } } Datatype::Bytes => { - let bd = Self::as_builder_mut::>(builder); + let bd = + Self::as_builder_mut::>(builder.as_mut()); let value = col .value .as_ref() @@ -327,47 +344,47 @@ impl Builder for DynRecordBuilder { } match datatype { Datatype::UInt8 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(u8::default()); } Datatype::UInt16 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(u16::default()); } Datatype::UInt32 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(u32::default()); } Datatype::UInt64 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(u64::default()); } Datatype::Int8 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(i8::default()); } Datatype::Int16 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(i16::default()); } Datatype::Int32 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(i32::default()); } Datatype::Int64 => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(i64::default()); } Datatype::String => { - Self::as_builder_mut::(builder) + Self::as_builder_mut::(builder.as_mut()) .append_value(String::default()); } Datatype::Boolean => { - Self::as_builder_mut::(builder) + Self::as_builder_mut::(builder.as_mut()) .append_value(bool::default()); } Datatype::Bytes => { - Self::as_builder_mut::>(builder) + Self::as_builder_mut::>(builder.as_mut()) .append_value(Vec::::default()); } } @@ -384,37 +401,46 @@ impl Builder for DynRecordBuilder { .fold(size, |acc, (builder, datatype)| { acc + match datatype { Datatype::UInt8 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::UInt16 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::UInt32 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::UInt64 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::Int8 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::Int16 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::Int32 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), Datatype::Int64 => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), + ), + Datatype::String => mem::size_of_val( + Self::as_builder::(builder.as_ref()).values_slice(), + ), + Datatype::Boolean => mem::size_of_val( + Self::as_builder::(builder.as_ref()).values_slice(), ), - Datatype::String => { - mem::size_of_val(Self::as_builder::(builder).values_slice()) - } - Datatype::Boolean => { - mem::size_of_val(Self::as_builder::(builder).values_slice()) - } Datatype::Bytes => mem::size_of_val( - Self::as_builder::>(builder).values_slice(), + Self::as_builder::>(builder.as_ref()) + .values_slice(), ), } }) @@ -437,7 +463,8 @@ impl Builder for DynRecordBuilder { match datatype { Datatype::UInt8 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::UInt8, @@ -449,7 +476,8 @@ impl Builder for DynRecordBuilder { } Datatype::UInt16 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::UInt16, @@ -461,7 +489,8 @@ impl Builder for DynRecordBuilder { } Datatype::UInt32 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::UInt32, @@ -473,7 +502,8 @@ impl Builder for DynRecordBuilder { } Datatype::UInt64 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::UInt64, @@ -485,7 +515,8 @@ impl Builder for DynRecordBuilder { } Datatype::Int8 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::Int8, @@ -497,7 +528,8 @@ impl Builder for DynRecordBuilder { } Datatype::Int16 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::Int16, @@ -509,7 +541,8 @@ impl Builder for DynRecordBuilder { } Datatype::Int32 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::Int32, @@ -521,7 +554,8 @@ impl Builder for DynRecordBuilder { } Datatype::Int64 => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::Int64, @@ -532,7 +566,8 @@ impl Builder for DynRecordBuilder { array_refs.push(value); } Datatype::String => { - let value = Arc::new(Self::as_builder_mut::(builder).finish()); + let value = + Arc::new(Self::as_builder_mut::(builder.as_mut()).finish()); columns.push(Column { datatype: Datatype::String, name: field.name().to_owned(), @@ -542,7 +577,8 @@ impl Builder for DynRecordBuilder { array_refs.push(value); } Datatype::Boolean => { - let value = Arc::new(Self::as_builder_mut::(builder).finish()); + let value = + Arc::new(Self::as_builder_mut::(builder.as_mut()).finish()); columns.push(Column { datatype: Datatype::Boolean, name: field.name().to_owned(), @@ -553,7 +589,8 @@ impl Builder for DynRecordBuilder { } Datatype::Bytes => { let value = Arc::new( - Self::as_builder_mut::>(builder).finish(), + Self::as_builder_mut::>(builder.as_mut()) + .finish(), ); columns.push(Column { datatype: Datatype::Bytes, @@ -594,27 +631,41 @@ impl DynRecordBuilder { let datatype = self.datatypes.get_mut(primary_key_index).unwrap(); let col = key.value; match datatype { - Datatype::UInt8 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::UInt16 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::UInt32 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::UInt64 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Int8 => Self::as_builder_mut::>(builder) + Datatype::UInt8 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::UInt16 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::UInt32 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::UInt64 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::Int8 => Self::as_builder_mut::>(builder.as_mut()) .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Int16 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Int32 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Int64 => Self::as_builder_mut::>(builder) - .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::String => Self::as_builder_mut::(builder) + Datatype::Int16 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::Int32 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::Int64 => { + Self::as_builder_mut::>(builder.as_mut()) + .append_value(*col.value.as_ref().downcast_ref::().unwrap()) + } + Datatype::String => Self::as_builder_mut::(builder.as_mut()) .append_value(col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Boolean => Self::as_builder_mut::(builder) + Datatype::Boolean => Self::as_builder_mut::(builder.as_mut()) .append_value(*col.value.as_ref().downcast_ref::().unwrap()), - Datatype::Bytes => Self::as_builder_mut::>(builder) + Datatype::Bytes => Self::as_builder_mut::>(builder.as_mut()) .append_value(col.value.as_ref().downcast_ref::>().unwrap()), }; } @@ -633,9 +684,3 @@ impl DynRecordBuilder { builder.as_any_mut().downcast_mut::().unwrap() } } - -unsafe impl Send for DynRecordBuilder {} -unsafe impl Sync for DynRecordBuilder {} - -unsafe impl Send for DynRecordImmutableArrays {} -unsafe impl Sync for DynRecordImmutableArrays {} diff --git a/src/record/runtime/column.rs b/src/record/runtime/column.rs index 9eb51ba..0b356a6 100644 --- a/src/record/runtime/column.rs +++ b/src/record/runtime/column.rs @@ -35,16 +35,18 @@ impl ColumnDesc { #[derive(Clone)] pub struct Column { pub datatype: Datatype, - pub value: Arc, + pub value: Arc, pub is_nullable: bool, pub name: String, } -unsafe impl Send for Column {} -unsafe impl Sync for Column {} - impl Column { - pub fn new(datatype: Datatype, name: String, value: Arc, is_nullable: bool) -> Self { + pub fn new( + datatype: Datatype, + name: String, + value: Arc, + is_nullable: bool, + ) -> Self { Self { datatype, name, @@ -102,7 +104,6 @@ impl PartialOrd for Column { } } -#[macro_export] macro_rules! implement_col { ([], $({$Type:ty, $Datatype:ident}), *) => { impl Ord for Column { @@ -168,7 +169,6 @@ macro_rules! implement_col { }; } -#[macro_export] macro_rules! implement_key_col { ($({$Type:ident, $Datatype:ident, $Array:ident}), *) => { impl Key for Column { @@ -224,7 +224,6 @@ impl<'r> KeyRef<'r> for Column { } } -#[macro_export] macro_rules! implement_decode_col { ([], $({$Type:ty, $Datatype:ident}), *) => { impl Decode for Column { @@ -248,8 +247,8 @@ macro_rules! implement_decode_col { DecodeError::Fusio(error) => error, DecodeError::Inner(error) => fusio::Error::Other(Box::new(error)), }, - )?) as Arc, - false => Arc::new(<$Type>::decode(reader).await?) as Arc, + )?) as Arc, + false => Arc::new(<$Type>::decode(reader).await?) as Arc, }, )* }; @@ -265,7 +264,6 @@ macro_rules! implement_decode_col { } } -#[macro_export] macro_rules! implement_encode_col { ([], $({$Type:ty, $Datatype:ident}), *) => { impl Encode for Column { @@ -374,7 +372,6 @@ impl From<&Column> for Field { } } -#[macro_export] macro_rules! for_datatype { ($macro:tt $(, $x:tt)*) => { $macro! { diff --git a/src/record/runtime/record.rs b/src/record/runtime/record.rs index 24c0ca0..3c11eb6 100644 --- a/src/record/runtime/record.rs +++ b/src/record/runtime/record.rs @@ -55,7 +55,7 @@ impl DynRecord { pub(crate) fn empty_record(column_descs: Vec, primary_index: usize) -> DynRecord { let mut columns = vec![]; for desc in column_descs.iter() { - let value: Arc = match desc.datatype { + let value: Arc = match desc.datatype { Datatype::UInt8 => match desc.is_nullable { true => Arc::>::new(None), false => Arc::new(u8::default()), @@ -274,9 +274,6 @@ impl Record for DynRecord { } } -unsafe impl Send for DynRecord {} -unsafe impl Sync for DynRecord {} - #[cfg(test)] pub(crate) mod test { use std::sync::Arc; diff --git a/src/record/runtime/record_ref.rs b/src/record/runtime/record_ref.rs index c1756ef..a9525e8 100644 --- a/src/record/runtime/record_ref.rs +++ b/src/record/runtime/record_ref.rs @@ -168,32 +168,32 @@ impl<'r> RecordRef<'r> for DynRecordRef<'r> { let v = col.as_string::(); if primary_index == idx - 2 { - Arc::new(v.value(offset).to_owned()) as Arc + Arc::new(v.value(offset).to_owned()) as Arc } else { let value = (!v.is_null(offset) && projection_mask.leaf_included(idx)) .then_some(v.value(offset).to_owned()); - Arc::new(value) as Arc + Arc::new(value) as Arc } } Datatype::Boolean => { let v = col.as_boolean(); if primary_index == idx - 2 { - Arc::new(v.value(offset).to_owned()) as Arc + Arc::new(v.value(offset).to_owned()) as Arc } else { let value = (!v.is_null(offset) && projection_mask.leaf_included(idx)) .then_some(v.value(offset).to_owned()); - Arc::new(value) as Arc + Arc::new(value) as Arc } } Datatype::Bytes => { let v = col.as_binary::(); if primary_index == idx - 2 { - Arc::new(v.value(offset).to_owned()) as Arc + Arc::new(v.value(offset).to_owned()) as Arc } else { let value = (!v.is_null(offset) && projection_mask.leaf_included(idx)) .then_some(v.value(offset).to_owned()); - Arc::new(value) as Arc + Arc::new(value) as Arc } } }; @@ -241,21 +241,18 @@ impl<'r> DynRecordRef<'r> { idx: usize, projection_mask: &'r parquet::arrow::ProjectionMask, primary: bool, - ) -> Arc + ) -> Arc where T: ArrowPrimitiveType, { let v = col.as_primitive::(); if primary { - Arc::new(v.value(offset)) as Arc + Arc::new(v.value(offset)) as Arc } else { let value = (!v.is_null(offset) && projection_mask.leaf_included(idx)) .then_some(v.value(offset)); - Arc::new(value) as Arc + Arc::new(value) as Arc } } } - -unsafe impl<'r> Send for DynRecordRef<'r> {} -unsafe impl<'r> Sync for DynRecordRef<'r> {}