kosiew commented on code in PR #24029:
URL: https://github.com/apache/datafusion/pull/24029#discussion_r3747356939
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -11544,4 +11547,28 @@ mod tests {
run_tests::<Decimal128Type>();
run_tests::<Decimal256Type>();
}
+
+ #[test]
+ fn test_new_list_nested_nullability_mismatch_issue_24022() {
+ // requested element type: Struct(n: Int32 nullable=true)
+ let requested_element_type =
+ DataType::Struct(Fields::from(vec![Field::new("n",
DataType::Int32, true)]));
+
+ // inferred from concrete values: Struct(n: Int32 nullable=false)
+ let inferred_field = Field::new("n", DataType::Int32, false);
+
+ let value = ScalarValue::Struct(Arc::new(StructArray::from(vec![(
+ Arc::new(inferred_field),
+ Arc::new(Int32Array::from(vec![1])) as ArrayRef,
+ )])));
+
+ let list = ScalarValue::new_list(&[value], &requested_element_type,
true);
Review Comment:
Thanks for adding this regression test. One small coverage gap remains here.
The test currently exercises `new_list` and checks the resulting type, but it
does not cover `new_list_from_iter` or `new_large_list`, or verify the
normalized child values. Since the casting behavior is shared across these
constructors, could we add a shared assertion that exercises all three and
checks both the declared nested child type and the child values? This is
non-blocking, but it would fully cover the invariant discussed in the earlier
review.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
