klion26 commented on code in PR #8141:
URL: https://github.com/apache/arrow-rs/pull/8141#discussion_r2299667136
##########
parquet-variant/src/builder.rs:
##########
@@ -97,6 +97,39 @@ impl ValueBuilder {
}
}
+/// Macro to generate the match statement for each append_variant,
try_append_variant, and
+/// append_variant_bytes -- they each have slightly different handling for
object and list handling.
+macro_rules! variant_append_value {
+ ($builder:expr, $value:expr, $object_pat:pat => $object_arm:expr,
$list_pat:pat => $list_arm:expr) => {
Review Comment:
nice!
##########
parquet-variant/src/builder.rs:
##########
@@ -3297,4 +3374,347 @@ mod tests {
.contains("Field name 'unknown_field' not found"));
}
}
+
+ #[test]
+ fn test_append_variant_bytes_round_trip() {
Review Comment:
Do we need to add a test case for the `Variant::List` match arm in
`append_variant_bytes`?
##########
parquet-variant/src/builder.rs:
##########
@@ -3297,4 +3374,347 @@ mod tests {
.contains("Field name 'unknown_field' not found"));
}
}
+
+ #[test]
+ fn test_append_variant_bytes_round_trip() {
+ // Create a complex variant with the normal builder
+ let mut builder = VariantBuilder::new();
+ {
+ let mut obj = builder.new_object();
+ obj.insert("name", "Alice");
+ obj.insert("age", 30i32);
+ {
+ let mut scores_list = obj.new_list("scores");
+ scores_list.append_value(95i32);
+ scores_list.append_value(87i32);
+ scores_list.append_value(92i32);
+ scores_list.finish();
+ }
+ {
+ let mut address = obj.new_object("address");
+ address.insert("street", "123 Main St");
+ address.insert("city", "Anytown");
+ address.finish().unwrap();
+ }
+ obj.finish().unwrap();
+ }
+ let (metadata, value1) = builder.finish();
+ let variant1 = Variant::try_new(&metadata, &value1).unwrap();
+
+ // Copy using the new bytes API
+ let metadata = VariantMetadata::new(&metadata);
+ let mut metadata = ReadOnlyMetadataBuilder::new(metadata);
+ let mut builder2 = ValueBuilder::new();
+ let state = ParentState::variant(&mut builder2, &mut metadata);
+ ValueBuilder::append_variant_bytes(state, variant1.clone());
+ let value2 = builder2.into_inner();
+
+ // The bytes should be identical, we merely copied them across.
+ assert_eq!(value1, value2);
+ }
+
+ #[test]
+ fn test_object_insert_bytes_subset() {
+ // Create an original object, making sure to inject the field names
we'll add later.
+ let mut builder = VariantBuilder::new().with_field_names(["new_field",
"another_field"]);
+ {
+ let mut obj = builder.new_object();
+ obj.insert("field1", "value1");
+ obj.insert("field2", 42i32);
+ obj.insert("field3", true);
+ obj.insert("field4", "value4");
+ obj.finish().unwrap();
+ }
+ let (metadata1, value1) = builder.finish();
+ let original_variant = Variant::try_new(&metadata1, &value1).unwrap();
+ let original_obj = original_variant.as_object().unwrap();
+
+ // Create a new object copying subset of fields interleaved with new
ones
+ let metadata2 = VariantMetadata::new(&metadata1);
+ let mut metadata2 = ReadOnlyMetadataBuilder::new(metadata2);
+ let mut builder2 = ValueBuilder::new();
+ let state = ParentState::variant(&mut builder2, &mut metadata2);
+ {
+ let mut obj = ObjectBuilder::new(state, true);
+
+ // Copy field1 using bytes API
+ obj.insert_bytes("field1", original_obj.get("field1").unwrap());
+
+ // Add new field
+ obj.insert("new_field", "new_value");
+
+ // Copy field3 using bytes API
+ obj.insert_bytes("field3", original_obj.get("field3").unwrap());
+
+ // Add another new field
+ obj.insert("another_field", 99i32);
+
+ // Copy field2 using bytes API
+ obj.insert_bytes("field2", original_obj.get("field2").unwrap());
+
+ obj.finish().unwrap();
+ }
+ let value2 = builder2.into_inner();
+ let result_variant = Variant::try_new(&metadata1, &value2).unwrap();
+ let result_obj = result_variant.as_object().unwrap();
+
+ // Verify the object contains expected fields
+ assert_eq!(result_obj.len(), 5);
+ assert_eq!(
+ result_obj.get("field1").unwrap().as_string().unwrap(),
+ "value1"
+ );
+ assert_eq!(result_obj.get("field2").unwrap().as_int32().unwrap(), 42);
+ assert!(result_obj.get("field3").unwrap().as_boolean().unwrap());
+ assert_eq!(
+ result_obj.get("new_field").unwrap().as_string().unwrap(),
+ "new_value"
+ );
+ assert_eq!(
+ result_obj.get("another_field").unwrap().as_int32().unwrap(),
+ 99
+ );
+ }
+
+ #[test]
+ fn test_list_append_bytes_subset() {
+ // Create an original list
+ let mut builder = VariantBuilder::new();
+ {
+ let mut list = builder.new_list();
+ list.append_value("item1");
+ list.append_value(42i32);
+ list.append_value(true);
+ list.append_value("item4");
+ list.append_value(1.234f64);
+ list.finish();
+ }
+ let (metadata1, value1) = builder.finish();
+ let original_variant = Variant::try_new(&metadata1, &value1).unwrap();
+ let original_list = original_variant.as_list().unwrap();
+
+ // Create a new list copying subset of elements interleaved with new
ones
+ let metadata2 = VariantMetadata::new(&metadata1);
+ let mut metadata2 = ReadOnlyMetadataBuilder::new(metadata2);
+ let mut builder2 = ValueBuilder::new();
+ let state = ParentState::variant(&mut builder2, &mut metadata2);
+ {
+ let mut list = ListBuilder::new(state, true);
+
+ // Copy first element using bytes API
+ list.append_value_bytes(original_list.get(0).unwrap());
+
+ // Add new element
+ list.append_value("new_item");
+
+ // Copy third element using bytes API
+ list.append_value_bytes(original_list.get(2).unwrap());
+
+ // Add another new element
+ list.append_value(99i32);
+
+ // Copy last element using bytes API
+ list.append_value_bytes(original_list.get(4).unwrap());
+
+ list.finish();
+ }
+ let value2 = builder2.into_inner();
+ let result_variant = Variant::try_new(&metadata1, &value2).unwrap();
+ let result_list = result_variant.as_list().unwrap();
+
+ // Verify the list contains expected elements
+ assert_eq!(result_list.len(), 5);
+ assert_eq!(result_list.get(0).unwrap().as_string().unwrap(), "item1");
+ assert_eq!(result_list.get(1).unwrap().as_string().unwrap(),
"new_item");
+ assert!(result_list.get(2).unwrap().as_boolean().unwrap());
+ assert_eq!(result_list.get(3).unwrap().as_int32().unwrap(), 99);
+ assert_eq!(result_list.get(4).unwrap().as_f64().unwrap(), 1.234);
+ }
+
+ #[test]
+ fn test_complex_nested_filtering_injection() {
Review Comment:
Nice test!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
