Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 60 additions & 1 deletion datafusion/common/src/scalar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub use struct_builder::ScalarStructBuilder;
///
/// In general, performance will be better using arrow [`Array`]s rather than
/// [`ScalarValue`], as it is far more efficient to process multiple values at
/// once (vecctorized processing).
/// once (vectorized processing).
///
/// # Example
/// ```
Expand Down Expand Up @@ -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() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug fix

write!(f, "NULL")?;
return Ok(());
}

let columns = struct_arr.columns();
let fields = struct_arr.fields();
let nulls = struct_arr.nulls();
Expand Down Expand Up @@ -3298,6 +3303,7 @@ mod tests {
as_string_array, as_struct_array, as_uint32_array, as_uint64_array,
};

use crate::assert_batches_eq;
use arrow::buffer::OffsetBuffer;
use arrow::compute::{is_null, kernels};
use arrow::datatypes::{ArrowNumericType, ArrowPrimitiveType};
Expand Down Expand Up @@ -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 = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"+---+",
"| s |",
"+---+",
"| |",
"+---+",
];
assert_batches_eq!(&expected, &[batch]);
}

#[test]
fn test_build_timestamp_millisecond_list() {
let values = vec![ScalarValue::TimestampMillisecond(Some(1), None)];
Expand Down
36 changes: 34 additions & 2 deletions datafusion/common/src/scalar/struct_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,40 @@ impl ScalarStructBuilder {
Self::default()
}

/// Return a new [`ScalarValue::Struct`] with the specified fields and a
/// single null value
/// Return a new [`ScalarValue::Struct`] with a single `null` value.
///
/// Note this is different from a struct where each of the specified fields
/// are null (e.g. `{a: NULL}`)
///
Comment on lines +44 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// # Example
///
/// ```rust
/// # use arrow::datatypes::{DataType, Field};
/// # use datafusion_common::scalar::ScalarStructBuilder;
/// let fields = vec![
/// Field::new("a", DataType::Int32, false),
/// ];
/// let sv = ScalarStructBuilder::new_null(fields);
/// // Note this is `NULL`, not `{a: NULL}`
/// assert_eq!(format!("{sv}"), "NULL");
///```
///
/// To create a struct where the *fields* are null, use `Self::new()` and
/// pass null values for each field:
///
/// ```rust
/// # use arrow::datatypes::{DataType, Field};
/// # use datafusion_common::scalar::{ScalarStructBuilder, ScalarValue};
/// // make a nullable field
/// let field = Field::new("a", DataType::Int32, true);
/// // add a null value for the "a" field
/// let sv = ScalarStructBuilder::new()
/// .with_scalar(field, ScalarValue::Int32(None))
/// .build()
/// .unwrap();
/// // value is not null, but field is
/// assert_eq!(format!("{sv}"), "{a:}");
/// ```
pub fn new_null(fields: impl IntoFields) -> ScalarValue {
DataType::Struct(fields.into()).try_into().unwrap()
}
Expand Down