Add Round trip tests for Array <--> ScalarValue #13777
Conversation
| // null array | ||
| Arc::new(NullArray::new(3)), | ||
| // dense union | ||
| /* Dense union fails due to https://github.com/apache/datafusion/issues/13762 |
There was a problem hiding this comment.
This is the test for #13762
It fails because the type of the produced array is UnionArray::Sparse rather than dense
datafusion/common/src/scalar/mod.rs
Outdated
| Arc::new(StringArray::from(vec![Some("foo"), None, Some("bar")])), | ||
| Arc::new(LargeStringArray::from(vec![Some("foo"), None, Some("bar")])), | ||
| Arc::new(StringViewArray::from(vec![Some("foo"), None, Some("bar")])), | ||
| // string dictionary fails due to XXX |
There was a problem hiding this comment.
Sorry -- that was a left over from when I though dictionary failed, and left an XXX in there as a placeholder when I filed a ticket
| // [ ] (empty list) | ||
| builder.append(true); | ||
| // Null | ||
| builder.values().append_value("?"); // irrelevant |
There was a problem hiding this comment.
it can be omitted afaict, see apache/arrow-rs#6775
datafusion/common/src/scalar/mod.rs
Outdated
| let values_builder = Int32Builder::new(); | ||
| let mut builder = FixedSizeListBuilder::new(values_builder, 3); | ||
|
|
||
| // [[0, 1, 2], null, [3, null, 5], [6, 7, null]] |
There was a problem hiding this comment.
6,7,null are not in the data. update comment
datafusion/common/src/scalar/mod.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// for each index in array, convert to to a scalar and back to an array and compare for equality |
| let array = scalar.to_array_of_size(1).unwrap(); | ||
| assert_eq!(array.len(), 1); | ||
| assert_eq!(array.data_type(), arr.data_type()); | ||
| assert_eq!(array.as_ref(), arr.slice(i, 1).as_ref()); |
There was a problem hiding this comment.
does this compare internal representation or logical value?
for example a null list can have non-zero offset or zero offset (see comment above & apache/arrow-rs#6775)
There was a problem hiding this comment.
It compares logical values
Specifically Array::slice returns a new Array (that is underneath just a different view of the same underlying data / buffers)
| @@ -5554,6 +5555,194 @@ mod tests { | |||
| assert_eq!(&array, &expected); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn round_trip() { | |||
There was a problem hiding this comment.
the code in this test is more or less copy/pasted from the arrow documentation
| let array = scalar.to_array_of_size(1).unwrap(); | ||
| assert_eq!(array.len(), 1); | ||
| assert_eq!(array.data_type(), arr.data_type()); | ||
| assert_eq!(array.as_ref(), arr.slice(i, 1).as_ref()); |
There was a problem hiding this comment.
It compares logical values
Specifically Array::slice returns a new Array (that is underneath just a different view of the same underlying data / buffers)
Which issue does this PR close?
Rationale for this change
When trying to write a unit test for #13762 I found there was no good place to test the round tripping of Array --> ScalarValue --> Array (which is what is done for constant values)
So I wrote some
What changes are included in this PR?
Add tests for this path, with ones that fail due to bugs commented out
Are these changes tested?
Yes, by CI
Are there any user-facing changes?
No - this are just tests