sdf-jkl commented on code in PR #10352:
URL: https://github.com/apache/arrow-rs/pull/10352#discussion_r3694063665
##########
parquet-variant/src/variant/object.rs:
##########
@@ -994,4 +1017,71 @@ mod tests {
let v2 = Variant::new_with_metadata(m, &v);
assert_eq!(v1, v2);
}
+
+ #[test]
+ fn test_object_rejects_duplicate_field_names() {
+ // An unsorted dictionary may legally hold duplicate entries, but an
object must not
+ // reference the same field name twice.
+ let metadata_bytes = vec![
+ 0b0000_0001,
+ 2, // dictionary size
+ 0, // "a"
+ 1, // "a"
+ 2,
+ b'a',
+ b'a',
+ ];
+ assert!(
+ !VariantMetadata::try_new(&metadata_bytes)
+ .unwrap()
+ .is_sorted()
+ );
+
+ let value_bytes = vec![
+ 0b0000_0010, // object header
+ 2, // num_elements
+ 0, // field id 0 -> "a"
+ 1, // field id 1 -> "a"
+ 0,
+ 1,
+ 2, // field offsets
+ 0b0000_1100, // true
Review Comment:
nit
```suggestion
0b0000_0100, // true
```
##########
parquet-variant/src/variant/metadata.rs:
##########
@@ -608,6 +612,19 @@ mod tests {
matches!(err, ArrowError::InvalidArgumentError(_)),
"unexpected error: {err:?}"
);
+
+ let bytes = &[
+ 0b0000_0001, // header: offset_size_minus_one=0, ordered=0,
version=1
+ 2,
+ 0x00,
+ 0x02,
+ 0x02, // an unsorted dict may hold an empty string anywhere
+ b'h',
+ b'i',
+ ];
+ let metadata = VariantMetadata::try_new(bytes).unwrap();
+ assert_eq!(&metadata[0], "hi");
+ assert_eq!(&metadata[1], "");
Review Comment:
Add a unit test for `2 Unsorted dictionary offsets not checked for UTF-8
character boundaries`? Rn it's only covered in proptests
--
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]
