Weijun-H commented on code in PR #23424:
URL: https://github.com/apache/datafusion/pull/23424#discussion_r3569867698
##########
datafusion/functions/src/string/common.rs:
##########
@@ -636,15 +636,10 @@ fn case_conversion_ascii_array<O: OffsetSizeTrait>(
let values = Buffer::from_vec(converted);
// Shift offsets from `start`-based to 0-based so they index into `values`.
- let offsets = if start == 0 {
- string_array.offsets().clone()
- } else {
- let s = O::usize_as(start);
- let rebased: Vec<O> = value_offsets.iter().map(|&o| o - s).collect();
- // SAFETY: subtracting a constant from monotonic offsets preserves
- // monotonicity, and `start` is the minimum offset, so no underflow.
- unsafe { OffsetBuffer::new_unchecked(ScalarBuffer::from(rebased)) }
- };
+ let offsets = string_array
+ .offsets()
+ .clone()
+ .subtract(string_array.offsets()[0]);
Review Comment:
Nice cleanup — this also replaces the unsafe `new_unchecked` path with the
checked subtract. Since the subtrahend is `offsets()[0]`, the internal `self[0]
>= rhs` assert can never fire, so the safety upgrade is free.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
