From 2ab3bb59283552ea45e6de546c92716ef5f7b3d4 Mon Sep 17 00:00:00 2001 From: askoa Date: Wed, 16 Nov 2022 20:38:39 -0500 Subject: [PATCH 1/4] remove skip_serializing_if to please postcard --- arrow-schema/src/datatype.rs | 8 ++++---- arrow-schema/src/field.rs | 4 ---- arrow-schema/src/schema.rs | 4 ---- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/arrow-schema/src/datatype.rs b/arrow-schema/src/datatype.rs index 90ae429422c..b0979f8714e 100644 --- a/arrow-schema/src/datatype.rs +++ b/arrow-schema/src/datatype.rs @@ -415,11 +415,11 @@ mod tests { assert_eq!( "{\"Struct\":[\ {\"name\":\"first_name\",\"data_type\":\"Utf8\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false,\"metadata\":{\"k\":\"v\"}},\ - {\"name\":\"last_name\",\"data_type\":\"Utf8\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false},\ + {\"name\":\"last_name\",\"data_type\":\"Utf8\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false,\"metadata\":{}},\ {\"name\":\"address\",\"data_type\":{\"Struct\":\ - [{\"name\":\"street\",\"data_type\":\"Utf8\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false},\ - {\"name\":\"zip\",\"data_type\":\"UInt16\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false}\ - ]},\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false}]}", + [{\"name\":\"street\",\"data_type\":\"Utf8\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false,\"metadata\":{}},\ + {\"name\":\"zip\",\"data_type\":\"UInt16\",\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false,\"metadata\":{}}\ + ]},\"nullable\":false,\"dict_id\":0,\"dict_is_ordered\":false,\"metadata\":{}}]}", serialized ); diff --git a/arrow-schema/src/field.rs b/arrow-schema/src/field.rs index b1de65e557f..325d3c0b29e 100644 --- a/arrow-schema/src/field.rs +++ b/arrow-schema/src/field.rs @@ -35,10 +35,6 @@ pub struct Field { dict_id: i64, dict_is_ordered: bool, /// A map of key-value pairs containing additional custom meta data. - #[cfg_attr( - feature = "serde", - serde(skip_serializing_if = "BTreeMap::is_empty", default) - )] metadata: BTreeMap, } diff --git a/arrow-schema/src/schema.rs b/arrow-schema/src/schema.rs index 519a8e089ae..460ff47e0f7 100644 --- a/arrow-schema/src/schema.rs +++ b/arrow-schema/src/schema.rs @@ -34,10 +34,6 @@ pub type SchemaRef = std::sync::Arc; pub struct Schema { pub fields: Vec, /// A map of key-value pairs containing additional meta data. - #[cfg_attr( - feature = "serde", - serde(skip_serializing_if = "HashMap::is_empty", default) - )] pub metadata: HashMap, } From 9ba22a1355d95eb35417146b8fd09add0e9eae50 Mon Sep 17 00:00:00 2001 From: askoa Date: Wed, 16 Nov 2022 20:49:02 -0500 Subject: [PATCH 2/4] add serde postcard roundtrip tests --- arrow-schema/Cargo.toml | 1 + arrow-schema/src/field.rs | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/arrow-schema/Cargo.toml b/arrow-schema/Cargo.toml index 3b809f23ed4..ab785f780b8 100644 --- a/arrow-schema/Cargo.toml +++ b/arrow-schema/Cargo.toml @@ -45,3 +45,4 @@ default = [] [dev-dependencies] serde_json = "1.0" +postcard = { version = "1.0.2", default-features = false, features = ["alloc"] } diff --git a/arrow-schema/src/field.rs b/arrow-schema/src/field.rs index 325d3c0b29e..a01e5c9109a 100644 --- a/arrow-schema/src/field.rs +++ b/arrow-schema/src/field.rs @@ -599,4 +599,37 @@ mod test { assert!(!field1.contains(&field2)); assert!(!field2.contains(&field1)); } -} + + #[cfg(feature = "serde")] + fn assert_binary_serde_round_trip(field: Field) { + let serialized = postcard::to_allocvec(&field).unwrap(); + let deserialized: Field = postcard::from_bytes(&serialized).unwrap(); + assert_eq!(field, deserialized) + } + + #[cfg(feature = "serde")] + #[test] + fn test_field_without_metadata_serde() { + let field = Field::new("name", DataType::Boolean, true); + assert_binary_serde_round_trip(field) + } + + #[cfg(feature = "serde")] + #[test] + fn test_field_with_empty_metadata_serde() { + let field = Field::new("name", DataType::Boolean, false) + .with_metadata(BTreeMap::new()); + + assert_binary_serde_round_trip(field) + } + + #[cfg(feature = "serde")] + #[test] + fn test_field_with_nonempty_metadata_serde() { + let mut metadata = BTreeMap::new(); + metadata.insert("hi".to_owned(), "".to_owned()); + let field = + Field::new("name", DataType::Boolean, false).with_metadata(metadata); + + assert_binary_serde_round_trip(field) + }} From 5041a74cacd404ca17ea7abf889fb4de0e4f871f Mon Sep 17 00:00:00 2001 From: askoa Date: Wed, 16 Nov 2022 20:51:23 -0500 Subject: [PATCH 3/4] fix formatting issues --- arrow-schema/src/field.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/arrow-schema/src/field.rs b/arrow-schema/src/field.rs index a01e5c9109a..da671de4cfd 100644 --- a/arrow-schema/src/field.rs +++ b/arrow-schema/src/field.rs @@ -617,8 +617,8 @@ mod test { #[cfg(feature = "serde")] #[test] fn test_field_with_empty_metadata_serde() { - let field = Field::new("name", DataType::Boolean, false) - .with_metadata(BTreeMap::new()); + let field = + Field::new("name", DataType::Boolean, false).with_metadata(BTreeMap::new()); assert_binary_serde_round_trip(field) } @@ -628,8 +628,8 @@ mod test { fn test_field_with_nonempty_metadata_serde() { let mut metadata = BTreeMap::new(); metadata.insert("hi".to_owned(), "".to_owned()); - let field = - Field::new("name", DataType::Boolean, false).with_metadata(metadata); + let field = Field::new("name", DataType::Boolean, false).with_metadata(metadata); assert_binary_serde_round_trip(field) - }} + } +} From 4031554dd0e74820f76ac62e96d8b3a8fec58e47 Mon Sep 17 00:00:00 2001 From: askoa Date: Mon, 21 Nov 2022 14:26:01 -0500 Subject: [PATCH 4/4] use bincode for serialization tests --- arrow-schema/Cargo.toml | 2 +- arrow-schema/src/field.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arrow-schema/Cargo.toml b/arrow-schema/Cargo.toml index ab785f780b8..d88632d1040 100644 --- a/arrow-schema/Cargo.toml +++ b/arrow-schema/Cargo.toml @@ -45,4 +45,4 @@ default = [] [dev-dependencies] serde_json = "1.0" -postcard = { version = "1.0.2", default-features = false, features = ["alloc"] } +bincode = { version = "1.3.3", default-features = false } diff --git a/arrow-schema/src/field.rs b/arrow-schema/src/field.rs index bb69c43ec6c..9eed03ed24e 100644 --- a/arrow-schema/src/field.rs +++ b/arrow-schema/src/field.rs @@ -653,8 +653,8 @@ mod test { #[cfg(feature = "serde")] fn assert_binary_serde_round_trip(field: Field) { - let serialized = postcard::to_allocvec(&field).unwrap(); - let deserialized: Field = postcard::from_bytes(&serialized).unwrap(); + let serialized = bincode::serialize(&field).unwrap(); + let deserialized: Field = bincode::deserialize(&serialized).unwrap(); assert_eq!(field, deserialized) } @@ -669,7 +669,7 @@ mod test { #[test] fn test_field_with_empty_metadata_serde() { let field = - Field::new("name", DataType::Boolean, false).with_metadata(BTreeMap::new()); + Field::new("name", DataType::Boolean, false).with_metadata(HashMap::new()); assert_binary_serde_round_trip(field) } @@ -677,7 +677,7 @@ mod test { #[cfg(feature = "serde")] #[test] fn test_field_with_nonempty_metadata_serde() { - let mut metadata = BTreeMap::new(); + let mut metadata = HashMap::new(); metadata.insert("hi".to_owned(), "".to_owned()); let field = Field::new("name", DataType::Boolean, false).with_metadata(metadata);