alamb commented on code in PR #9238:
URL: https://github.com/apache/arrow-datafusion/pull/9238#discussion_r1491022390


##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -5690,6 +5696,59 @@ mod tests {
         check_array(array);
     }
 
+    #[test]
+    fn test_struct_display() {
+        let field_a = Field::new("a", DataType::Int32, true);
+        let field_b = Field::new("b", DataType::Utf8, true);
+
+        let s = ScalarStructBuilder::new()
+            .with_scalar(field_a, ScalarValue::from(1i32))
+            .with_scalar(field_b, ScalarValue::Utf8(None))
+            .build()
+            .unwrap();
+
+        assert_eq!(s.to_string(), "{a:1,b:}");
+
+        let ScalarValue::Struct(arr) = s else {
+            panic!("Expected struct");
+        };
+
+        //verify compared to arrow display
+        let batch = RecordBatch::try_from_iter(vec![("s", arr as _)]).unwrap();
+        let expected = [
+            "+-------------+",
+            "| s           |",
+            "+-------------+",
+            "| {a: 1, b: } |",
+            "+-------------+",
+        ];
+        assert_batches_eq!(&expected, &[batch]);
+    }
+
+    #[test]
+    fn test_struct_display_null() {
+        let fields = vec![Field::new("a", DataType::Int32, false)];
+        let s = ScalarStructBuilder::new_null(fields);
+        assert_eq!(s.to_string(), "NULL");
+
+        let ScalarValue::Struct(arr) = s else {
+            panic!("Expected struct");
+        };
+
+        //verify compared to arrow display
+        let batch = RecordBatch::try_from_iter(vec![("s", arr as _)]).unwrap();
+
+        #[rustfmt::skip]
+        let expected = [

Review Comment:
   This shows that `new_null` really does create a null value (and thus the 
change to `impl Display` is correct)



##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -3103,6 +3103,11 @@ impl fmt::Display for ScalarValue {
                 // ScalarValue Struct should always have a single element
                 assert_eq!(struct_arr.len(), 1);
 
+                if struct_arr.null_count() == struct_arr.len() {

Review Comment:
   This is the bug fix



-- 
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