Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate nullable flag on StructArray fields #1611

Closed
jhorstmann opened this issue Apr 23, 2022 · 1 comment
Closed

Validate nullable flag on StructArray fields #1611

jhorstmann opened this issue Apr 23, 2022 · 1 comment
Labels

Comments

@jhorstmann
Copy link
Contributor

Describe the bug

Extracted from a discussion on the equals kernels

It is currently very easy to get inconsistant state between the nullable flag on StructArray fields and the presence of their validity bitmap. This can lead to very difficult to debug issues, as for example the equality comparison of StructArray includes this flag, but it is not printed by the Debug impl, for example in assertions.

To Reproduce

Example of a confusing test:

    #[test]
    fn compare_nullable_struct_array_fields() {
        let array1 = StructArray::try_from(vec![(
            "a",
            Arc::new(Int32Array::from(vec![1, 2, 3])) as ArrayRef,
        )])
        .unwrap();
        let array2 = StructArray::try_from(vec![(
            "a",
            Arc::new(Int32Array::from(vec![Some(1), Some(2), Some(3)])) as ArrayRef,
        )])
        .unwrap();
        // assert_eq!(array1.data_type(), array2.data_type());
        assert_eq!(&array1, &array2);
    }
panicked at 'assertion failed: `(left == right)`
  left: `StructArray
[
-- child 0: "a" (Int32)
PrimitiveArray<Int32>
[
  1,
  2,
  3,
]
]`,
 right: `StructArray
[
-- child 0: "a" (Int32)
PrimitiveArray<Int32>
[
  1,
  2,
  3,
]
]`'

Expected behavior

  • if a field is not nullable it should not have a validity bitmap
  • a StructArray itself should only have a validity bitmap if all its fields are nullable
  • maybe StructArray::from(Vec<(&str, ArrayRef>) should always set the nullable flag?

A clear and concise description of what you expected to happen.

Additional context

Alternatives could be to ignore the nullable flag when comparing arrays

@tustvold
Copy link
Contributor

Closed by #3244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants