rluvaton commented on code in PR #10272:
URL: https://github.com/apache/arrow-rs/pull/10272#discussion_r3523798298


##########
arrow-ipc/src/reader.rs:
##########
@@ -2954,13 +2954,13 @@ mod tests {
         let bin_view_array = Arc::new(BinaryViewArray::from_iter(bin_values));
         let utf8_view_array = 
Arc::new(StringViewArray::from_iter(utf8_values));
 
-        let key_dict_keys = Int8Array::from_iter_values([0, 0, 1, 2, 0, 1, 3]);
+        let key_dict_keys = Int8Array::from_iter_values([0, 0, 2, 2, 0, 2, 3]);
         let key_dict_array = DictionaryArray::new(key_dict_keys, 
utf8_view_array.clone());
         #[allow(deprecated)]
         let keys_field = Arc::new(Field::new_dict(
             "keys",
             DataType::Dictionary(Box::new(DataType::Int8), 
Box::new(DataType::Utf8View)),
-            true,
+            false,
             1,
             false,
         ));

Review Comment:
   both tests failed,
   
   the pr that disallowed nullable entries keys kept the nullable key even if 
not allowed on purpose
   
   - https://github.com/apache/arrow-rs/pull/4808#discussion_r1320669828
   
   but I looked at the original pr that added that test and it did not seem to 
be on purpose that the keys have nulls:
   - https://github.com/apache/arrow-rs/pull/1583
   
   so I modified the 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]

Reply via email to