scovich commented on code in PR #8354:
URL: https://github.com/apache/arrow-rs/pull/8354#discussion_r2984638988
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -92,13 +89,8 @@ fn take_list_like_index_as_shredding_state<L: ListLikeArray
+ 'static>(
ArrowError::ComputeError("List-like index does not fit into
u64".into())
Review Comment:
If we're going for jsonpath semantics, should array index overflow also
produce NULL instead of error?
On the other hand, one could argue that `foo[18446744073709551616]` (a
65-bit number) is an invalid jsonpath because it could never produce a valid
array element, regardless of the underlying data?
But on the other hand, these overflow errors are not due to the jsonpath
syntax at all -- they're due to overflow in the internal data structures. But
actually... how could these overflows ever arise in the first place? We know
`row_range.end` is a valid `usize`, as are `index` and `row_range.len()`, and
because `index < len`, we know `row_range.begin + index` is _also_ a valid
`usize`. So the first `checked_add` can never fail. And the conversion from
`usize` to `u64` also cannot fail on any hardware that exists today (I don't
know of any hardware with 128-bit pointers, at least).
Overall, I wonder if the whole thing is just:
```rust
let index = (index < row_range.len()).then(|| row_range.start + index as
u64);
take_indices.push(index);
```
--
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]
