Skip to content

Commit 78212bf

Browse files
alambkylebarron
andauthored
Docs: Clarify that Array::value does not check for nulls (#8065)
# Which issue does this PR close? We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. - Related to #8021 # Rationale for this change As part of the review in #8021, @scovich and I were discussing how `VariantArray::value` should behave in the presence of nulls: #8021 (comment) > Suggest to make this return Option<Variant> so callers don't have to check for null themselves. I realized it might not be 100% clear that the existing convention in this crate was that `value()` methods did not check for nulls / return `Option`. I think we should document it better # What changes are included in this PR? Explicitly document that `value` methods do not check for nulls and explain what happens when they are used on null values # Are these changes tested? Yes, by CI # Are there any user-facing changes? Additional documentation. No behavior changes --------- Co-authored-by: Kyle Barron <[email protected]>
1 parent e531df7 commit 78212bf

File tree

11 files changed

+89
-3
lines changed

11 files changed

+89
-3
lines changed

arrow-array/src/array/boolean_array.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,13 +178,20 @@ impl BooleanArray {
178178

179179
/// Returns the boolean value at index `i`.
180180
///
181+
/// Note: This method does not check for nulls and the value is arbitrary
182+
/// if [`is_null`](Self::is_null) returns true for the index.
183+
///
181184
/// # Safety
182185
/// This doesn't check bounds, the caller must ensure that index < self.len()
183186
pub unsafe fn value_unchecked(&self, i: usize) -> bool {
184187
self.values.value_unchecked(i)
185188
}
186189

187190
/// Returns the boolean value at index `i`.
191+
///
192+
/// Note: This method does not check for nulls and the value is arbitrary
193+
/// if [`is_null`](Self::is_null) returns true for the index.
194+
///
188195
/// # Panics
189196
/// Panics if index `i` is out of bounds
190197
pub fn value(&self, i: usize) -> bool {

arrow-array/src/array/byte_array.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ impl<T: ByteArrayType> GenericByteArray<T> {
276276
}
277277

278278
/// Returns the element at index `i`
279+
///
280+
/// Note: This method does not check for nulls and the value is arbitrary
281+
/// if [`is_null`](Self::is_null) returns true for the index.
282+
///
279283
/// # Safety
280284
/// Caller is responsible for ensuring that the index is within the bounds of the array
281285
pub unsafe fn value_unchecked(&self, i: usize) -> &T::Native {
@@ -304,6 +308,10 @@ impl<T: ByteArrayType> GenericByteArray<T> {
304308
}
305309

306310
/// Returns the element at index `i`
311+
///
312+
/// Note: This method does not check for nulls and the value is arbitrary
313+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
314+
///
307315
/// # Panics
308316
/// Panics if index `i` is out of bounds.
309317
pub fn value(&self, i: usize) -> &T::Native {

arrow-array/src/array/byte_view_array.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
296296
}
297297

298298
/// Returns the element at index `i`
299+
///
300+
/// Note: This method does not check for nulls and the value is arbitrary
301+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
302+
///
299303
/// # Panics
300304
/// Panics if index `i` is out of bounds.
301305
pub fn value(&self, i: usize) -> &T::Native {
@@ -312,6 +316,9 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
312316

313317
/// Returns the element at index `i` without bounds checking
314318
///
319+
/// Note: This method does not check for nulls and the value is arbitrary
320+
/// if [`is_null`](Self::is_null) returns true for the index.
321+
///
315322
/// # Safety
316323
///
317324
/// Caller is responsible for ensuring that the index is within the bounds

arrow-array/src/array/fixed_size_binary_array.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ impl FixedSizeBinaryArray {
135135
}
136136

137137
/// Returns the element at index `i` as a byte slice.
138+
///
139+
/// Note: This method does not check for nulls and the value is arbitrary
140+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
141+
///
138142
/// # Panics
139143
/// Panics if index `i` is out of bounds.
140144
pub fn value(&self, i: usize) -> &[u8] {
@@ -155,8 +159,14 @@ impl FixedSizeBinaryArray {
155159
}
156160

157161
/// Returns the element at index `i` as a byte slice.
162+
///
163+
/// Note: This method does not check for nulls and the value is arbitrary
164+
/// if [`is_null`](Self::is_null) returns true for the index.
165+
///
158166
/// # Safety
159-
/// Caller is responsible for ensuring that the index is within the bounds of the array
167+
///
168+
/// Caller is responsible for ensuring that the index is within the bounds
169+
/// of the array
160170
pub unsafe fn value_unchecked(&self, i: usize) -> &[u8] {
161171
let offset = i + self.offset();
162172
let pos = self.value_offset_at(offset);

arrow-array/src/array/fixed_size_list_array.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,12 @@ impl FixedSizeListArray {
243243
}
244244

245245
/// Returns ith value of this list array.
246+
///
247+
/// Note: This method does not check for nulls and the value is arbitrary
248+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
249+
///
250+
/// # Panics
251+
/// Panics if index `i` is out of bounds
246252
pub fn value(&self, i: usize) -> ArrayRef {
247253
self.values
248254
.slice(self.value_offset_at(i), self.value_length() as usize)

arrow-array/src/array/list_array.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,10 @@ impl<OffsetSize: OffsetSizeTrait> GenericListArray<OffsetSize> {
327327
}
328328

329329
/// Returns ith value of this list array.
330+
///
331+
/// Note: This method does not check for nulls and the value is arbitrary
332+
/// if [`is_null`](Self::is_null) returns true for the index.
333+
///
330334
/// # Safety
331335
/// Caller must ensure that the index is within the array bounds
332336
pub unsafe fn value_unchecked(&self, i: usize) -> ArrayRef {
@@ -336,6 +340,12 @@ impl<OffsetSize: OffsetSizeTrait> GenericListArray<OffsetSize> {
336340
}
337341

338342
/// Returns ith value of this list array.
343+
///
344+
/// Note: This method does not check for nulls and the value is arbitrary
345+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
346+
///
347+
/// # Panics
348+
/// Panics if index `i` is out of bounds
339349
pub fn value(&self, i: usize) -> ArrayRef {
340350
let end = self.value_offsets()[i + 1].as_usize();
341351
let start = self.value_offsets()[i].as_usize();

arrow-array/src/array/list_view_array.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ impl<OffsetSize: OffsetSizeTrait> GenericListViewArray<OffsetSize> {
283283
}
284284

285285
/// Returns ith value of this list view array.
286+
///
287+
/// Note: This method does not check for nulls and the value is arbitrary
288+
/// if [`is_null`](Self::is_null) returns true for the index.
289+
///
286290
/// # Safety
287291
/// Caller must ensure that the index is within the array bounds
288292
pub unsafe fn value_unchecked(&self, i: usize) -> ArrayRef {
@@ -292,6 +296,10 @@ impl<OffsetSize: OffsetSizeTrait> GenericListViewArray<OffsetSize> {
292296
}
293297

294298
/// Returns ith value of this list view array.
299+
///
300+
/// Note: This method does not check for nulls and the value is arbitrary
301+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
302+
///
295303
/// # Panics
296304
/// Panics if the index is out of bounds
297305
pub fn value(&self, i: usize) -> ArrayRef {

arrow-array/src/array/map_array.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ impl MapArray {
185185

186186
/// Returns ith value of this map array.
187187
///
188+
/// Note: This method does not check for nulls and the value is arbitrary
189+
/// if [`is_null`](Self::is_null) returns true for the index.
190+
///
188191
/// # Safety
189192
/// Caller must ensure that the index is within the array bounds
190193
pub unsafe fn value_unchecked(&self, i: usize) -> StructArray {
@@ -197,6 +200,12 @@ impl MapArray {
197200
/// Returns ith value of this map array.
198201
///
199202
/// This is a [`StructArray`] containing two fields
203+
///
204+
/// Note: This method does not check for nulls and the value is arbitrary
205+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
206+
///
207+
/// # Panics
208+
/// Panics if index `i` is out of bounds
200209
pub fn value(&self, i: usize) -> StructArray {
201210
let end = self.value_offsets()[i + 1] as usize;
202211
let start = self.value_offsets()[i] as usize;

arrow-array/src/array/primitive_array.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,9 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
720720

721721
/// Returns the primitive value at index `i`.
722722
///
723+
/// Note: This method does not check for nulls and the value is arbitrary
724+
/// if [`is_null`](Self::is_null) returns true for the index.
725+
///
723726
/// # Safety
724727
///
725728
/// caller must ensure that the passed in offset is less than the array len()
@@ -729,6 +732,10 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
729732
}
730733

731734
/// Returns the primitive value at index `i`.
735+
///
736+
/// Note: This method does not check for nulls and the value is arbitrary
737+
/// if [`is_null`](Self::is_null) returns true for the index.
738+
///
732739
/// # Panics
733740
/// Panics if index `i` is out of bounds
734741
#[inline]
@@ -1235,6 +1242,8 @@ where
12351242
///
12361243
/// If a data type cannot be converted to `NaiveDateTime`, a `None` is returned.
12371244
/// A valid value is expected, thus the user should first check for validity.
1245+
///
1246+
/// See notes on [`PrimitiveArray::value`] regarding nulls and panics
12381247
pub fn value_as_datetime(&self, i: usize) -> Option<NaiveDateTime> {
12391248
as_datetime::<T>(i64::from(self.value(i)))
12401249
}
@@ -1243,27 +1252,35 @@ where
12431252
///
12441253
/// functionally it is same as `value_as_datetime`, however it adds
12451254
/// the passed tz to the to-be-returned NaiveDateTime
1255+
///
1256+
/// See notes on [`PrimitiveArray::value`] regarding nulls and panics
12461257
pub fn value_as_datetime_with_tz(&self, i: usize, tz: Tz) -> Option<DateTime<Tz>> {
12471258
as_datetime_with_timezone::<T>(i64::from(self.value(i)), tz)
12481259
}
12491260

12501261
/// Returns value as a chrono `NaiveDate` by using `Self::datetime()`
12511262
///
12521263
/// If a data type cannot be converted to `NaiveDate`, a `None` is returned
1264+
///
1265+
/// See notes on [`PrimitiveArray::value`] regarding nulls and panics
12531266
pub fn value_as_date(&self, i: usize) -> Option<NaiveDate> {
12541267
self.value_as_datetime(i).map(|datetime| datetime.date())
12551268
}
12561269

12571270
/// Returns a value as a chrono `NaiveTime`
12581271
///
12591272
/// `Date32` and `Date64` return UTC midnight as they do not have time resolution
1273+
///
1274+
/// See notes on [`PrimitiveArray::value`] regarding nulls and panics
12601275
pub fn value_as_time(&self, i: usize) -> Option<NaiveTime> {
12611276
as_time::<T>(i64::from(self.value(i)))
12621277
}
12631278

12641279
/// Returns a value as a chrono `Duration`
12651280
///
12661281
/// If a data type cannot be converted to `Duration`, a `None` is returned
1282+
///
1283+
/// See notes on [`PrimitiveArray::value`] regarding nulls and panics
12671284
pub fn value_as_duration(&self, i: usize) -> Option<Duration> {
12681285
as_duration::<T>(i64::from(self.value(i)))
12691286
}

arrow-array/src/array/union_array.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,10 @@ impl UnionArray {
287287
}
288288

289289
/// Returns the array's value at index `i`.
290+
///
291+
/// Note: This method does not check for nulls and the value is arbitrary
292+
/// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
293+
///
290294
/// # Panics
291295
/// Panics if index `i` is out of bounds
292296
pub fn value(&self, i: usize) -> ArrayRef {

0 commit comments

Comments
 (0)