tustvold commented on code in PR #2579:
URL: https://github.com/apache/arrow-rs/pull/2579#discussion_r954132221
##########
arrow/src/array/array.rs:
##########
@@ -339,14 +339,44 @@ impl<'a, T: Array> Array for &'a T {
pub trait ArrayAccessor: Array {
type Item: Send + Sync;
+ /// `true` if null indices, i.e. where [`Array::is_null`], have a defined
value. The exact
+ /// value may be unspecified, but it must not be undefined.
+ ///
+ /// This allows for optimised kernels that can process the null mask
separately
+ /// from processing the value data.
+ ///
+ /// For example, the value of a [`PrimitiveArray`] is always well defined
as it always
+ /// corresponds to an initialized slot in the underlying buffer, and all
bit patterns
+ /// correspond to a valid primitive value.
+ ///
+ /// An example where nulls are not well defined is a [`DictionaryArray`].
This is because
+ /// the key at a null index is unspecified, and consequently may exceed
the bounds of the
+ /// values array.
+ ///
+ const NULLS_DEFINED: bool;
Review Comment:
I don't especially like that we end up polluting everything just because
dictionaries have strange null behaviour...
The more I think about it the more odd it is that DictionaryArray even
supports a null mask, given the values has a null mask...
##########
arrow/src/array/array.rs:
##########
@@ -339,14 +339,44 @@ impl<'a, T: Array> Array for &'a T {
pub trait ArrayAccessor: Array {
type Item: Send + Sync;
+ /// `true` if null indices, i.e. where [`Array::is_null`], have a defined
value. The exact
+ /// value may be unspecified, but it must not be undefined.
+ ///
+ /// This allows for optimised kernels that can process the null mask
separately
+ /// from processing the value data.
+ ///
+ /// For example, the value of a [`PrimitiveArray`] is always well defined
as it always
+ /// corresponds to an initialized slot in the underlying buffer, and all
bit patterns
+ /// correspond to a valid primitive value.
+ ///
+ /// An example where nulls are not well defined is a [`DictionaryArray`].
This is because
+ /// the key at a null index is unspecified, and consequently may exceed
the bounds of the
+ /// values array.
+ ///
+ const NULLS_DEFINED: bool;
Review Comment:
I don't especially like that we end up polluting everything just because
dictionaries have strange null behaviour...
--
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]
