Skip to content

Commit 057583d

Browse files
martin-gmartin-g
authored
chore: Use an enum to express the different kinds of nullability in an array (#18048)
* chore: Use an enum to express the different kinds of nullability in an array Follow-up of #17726 (review) * Use the Nulls enum also in .../multi_group_by/primitive.rs * Use the Nulls enum in .../multi_group_by/bytes[_view].rs as well
1 parent 6d3854f commit 057583d

File tree

5 files changed

+44
-27
lines changed

5 files changed

+44
-27
lines changed

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/boolean.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
use std::sync::Arc;
1919

20+
use crate::aggregates::group_values::multi_group_by::Nulls;
2021
use crate::aggregates::group_values::multi_group_by::{nulls_equal_to, GroupColumn};
2122
use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
2223
use arrow::array::{Array as _, ArrayRef, AsArray, BooleanArray, BooleanBufferBuilder};
@@ -115,15 +116,15 @@ impl<const NULLABLE: bool> GroupColumn for BooleanGroupValueBuilder<NULLABLE> {
115116
let null_count = array.null_count();
116117
let num_rows = array.len();
117118
let all_null_or_non_null = if null_count == 0 {
118-
Some(true)
119+
Nulls::None
119120
} else if null_count == num_rows {
120-
Some(false)
121+
Nulls::All
121122
} else {
122-
None
123+
Nulls::Some
123124
};
124125

125126
match (NULLABLE, all_null_or_non_null) {
126-
(true, None) => {
127+
(true, Nulls::Some) => {
127128
for &row in rows {
128129
if array.is_null(row) {
129130
self.nulls.append(true);
@@ -135,14 +136,14 @@ impl<const NULLABLE: bool> GroupColumn for BooleanGroupValueBuilder<NULLABLE> {
135136
}
136137
}
137138

138-
(true, Some(true)) => {
139+
(true, Nulls::None) => {
139140
self.nulls.append_n(rows.len(), false);
140141
for &row in rows {
141142
self.buffer.append(arr.value(row));
142143
}
143144
}
144145

145-
(true, Some(false)) => {
146+
(true, Nulls::All) => {
146147
self.nulls.append_n(rows.len(), true);
147148
self.buffer.append_n(rows.len(), bool::default());
148149
}

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use crate::aggregates::group_values::multi_group_by::{nulls_equal_to, GroupColumn};
18+
use crate::aggregates::group_values::multi_group_by::{
19+
nulls_equal_to, GroupColumn, Nulls,
20+
};
1921
use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
2022
use arrow::array::{
2123
types::GenericStringType, Array, ArrayRef, AsArray, BufferBuilder,
@@ -138,28 +140,28 @@ where
138140
let null_count = array.null_count();
139141
let num_rows = array.len();
140142
let all_null_or_non_null = if null_count == 0 {
141-
Some(true)
143+
Nulls::None
142144
} else if null_count == num_rows {
143-
Some(false)
145+
Nulls::All
144146
} else {
145-
None
147+
Nulls::Some
146148
};
147149

148150
match all_null_or_non_null {
149-
None => {
151+
Nulls::Some => {
150152
for &row in rows {
151153
self.append_val_inner::<B>(array, row)?
152154
}
153155
}
154156

155-
Some(true) => {
157+
Nulls::None => {
156158
self.nulls.append_n(rows.len(), false);
157159
for &row in rows {
158160
self.do_append_val_inner(arr, row)?;
159161
}
160162
}
161163

162-
Some(false) => {
164+
Nulls::All => {
163165
self.nulls.append_n(rows.len(), true);
164166

165167
let new_len = self.offsets.len() + rows.len();

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use crate::aggregates::group_values::multi_group_by::{nulls_equal_to, GroupColumn};
18+
use crate::aggregates::group_values::multi_group_by::{
19+
nulls_equal_to, GroupColumn, Nulls,
20+
};
1921
use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
2022
use arrow::array::{make_view, Array, ArrayRef, AsArray, ByteView, GenericByteViewArray};
2123
use arrow::buffer::{Buffer, ScalarBuffer};
@@ -145,28 +147,28 @@ impl<B: ByteViewType> ByteViewGroupValueBuilder<B> {
145147
let null_count = array.null_count();
146148
let num_rows = array.len();
147149
let all_null_or_non_null = if null_count == 0 {
148-
Some(true)
150+
Nulls::None
149151
} else if null_count == num_rows {
150-
Some(false)
152+
Nulls::All
151153
} else {
152-
None
154+
Nulls::Some
153155
};
154156

155157
match all_null_or_non_null {
156-
None => {
158+
Nulls::Some => {
157159
for &row in rows {
158160
self.append_val_inner(array, row);
159161
}
160162
}
161163

162-
Some(true) => {
164+
Nulls::None => {
163165
self.nulls.append_n(rows.len(), false);
164166
for &row in rows {
165167
self.do_append_val_inner(arr, row);
166168
}
167169
}
168170

169-
Some(false) => {
171+
Nulls::All => {
170172
self.nulls.append_n(rows.len(), true);
171173
let new_len = self.views.len() + rows.len();
172174
self.views.resize(new_len, 0);

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,16 @@ fn supported_type(data_type: &DataType) -> bool {
12501250
)
12511251
}
12521252

1253+
///Shows how many `null`s there are in an array
1254+
enum Nulls {
1255+
/// All array items are `null`s
1256+
All,
1257+
/// There are both `null`s and non-`null`s in the array items
1258+
Some,
1259+
/// There are no `null`s in the array items
1260+
None,
1261+
}
1262+
12531263
#[cfg(test)]
12541264
mod tests {
12551265
use std::{collections::HashMap, sync::Arc};

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/primitive.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use crate::aggregates::group_values::multi_group_by::{nulls_equal_to, GroupColumn};
18+
use crate::aggregates::group_values::multi_group_by::{
19+
nulls_equal_to, GroupColumn, Nulls,
20+
};
1921
use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
2022
use arrow::array::ArrowNativeTypeOp;
2123
use arrow::array::{cast::AsArray, Array, ArrayRef, ArrowPrimitiveType, PrimitiveArray};
@@ -132,15 +134,15 @@ impl<T: ArrowPrimitiveType, const NULLABLE: bool> GroupColumn
132134
let null_count = array.null_count();
133135
let num_rows = array.len();
134136
let all_null_or_non_null = if null_count == 0 {
135-
Some(true)
137+
Nulls::None
136138
} else if null_count == num_rows {
137-
Some(false)
139+
Nulls::All
138140
} else {
139-
None
141+
Nulls::Some
140142
};
141143

142144
match (NULLABLE, all_null_or_non_null) {
143-
(true, None) => {
145+
(true, Nulls::Some) => {
144146
for &row in rows {
145147
if array.is_null(row) {
146148
self.nulls.append(true);
@@ -152,14 +154,14 @@ impl<T: ArrowPrimitiveType, const NULLABLE: bool> GroupColumn
152154
}
153155
}
154156

155-
(true, Some(true)) => {
157+
(true, Nulls::None) => {
156158
self.nulls.append_n(rows.len(), false);
157159
for &row in rows {
158160
self.group_values.push(arr.value(row));
159161
}
160162
}
161163

162-
(true, Some(false)) => {
164+
(true, Nulls::All) => {
163165
self.nulls.append_n(rows.len(), true);
164166
self.group_values
165167
.extend(iter::repeat_n(T::default_value(), rows.len()));

0 commit comments

Comments
 (0)