Jefffrey commented on code in PR #9373:
URL: https://github.com/apache/arrow-rs/pull/9373#discussion_r2787183020


##########
arrow-data/src/transform/mod.rs:
##########
@@ -189,12 +189,14 @@ impl std::fmt::Debug for MutableArrayData<'_> {
 }
 
 /// Builds an extend that adds `offset` to the source primitive
-/// Additionally validates that `max` fits into the
-/// the underlying primitive returning None if not
+/// Additionally validates that the maximum index fits into the underlying 
primitive type
 fn build_extend_dictionary(array: &ArrayData, offset: usize, max: usize) -> 
Option<Extend<'_>> {
     macro_rules! validate_and_build {
         ($dt: ty) => {{
-            let _: $dt = max.try_into().ok()?;
+            if max > 0 {
+                let max_index = max - 1;

Review Comment:
   Personally I'm of the mind to fix this at the callsite; or rename `max` here 
since I assume `max` was meant to represent the `max` offset but if we need to 
adjust it after the fact it becomes a little confusing what exactly `max` is 
meant to be here (without looking inside the function)



-- 
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]

Reply via email to