meztez opened a new pull request, #933:
URL: https://github.com/apache/arrow-nanoarrow/pull/933
Fixes #932.
While investigating why converting a `list<int64>` array to a
`vctrs::list_of(ptype = bit64::integer64())` target silently produced wrong
values (not just a lossy-precision warning), I found two separate, independent
bugs, both in `r/`:
**1. `as_nanoarrow_array.list()` corrupted `integer64` list elements while
building the child array**
It built the flattened child vector with:
```r
child <- unlist(x, recursive = FALSE, use.names = FALSE)
```
`unlist()` strips the `integer64` S3 class from each element before
concatenating, so the elements are concatenated as plain `numeric`,
reinterpreting the raw 64-bit integer bit pattern as a double bit pattern. The
resulting child array is corrupted before conversion even runs. Fixed by using
`do.call(c, x)`, which dispatches to `c.integer64` (and any other registered
`c()` S3 method) and preserves the class/underlying representation.
**2. `nanoarrow_materialize_int64()` used the wrong buffer view for pointer
arithmetic**
```c
case NANOARROW_TYPE_INT64:
memcpy(result + dst->offset,
src->array_view->buffer_views[1].data.as_int32 + raw_src_offset,
dst->length * sizeof(int64_t));
```
`raw_src_offset` is added to `data.as_int32`, so the offset is scaled by 4
bytes instead of 8. Any int64 array/slice with a nonzero starting offset — such
as the per-row child slices nanoarrow builds internally when materializing a
`list<int64>` column — reads from the wrong memory location. Offset-0
conversions (the case covered by existing tests) are unaffected, which is
presumably why this slipped through since it was introduced in #293.
Both bugs needed fixing to correctly round-trip a `list<int64>` through
`bit64::integer64`; either one alone still corrupts values.
**Testing**
Added regression tests to `test-as-array.R` and `test-convert-array.R`. I
confirmed both new tests fail against the pre-fix code and pass after the fix
(see repro below), and that the full existing `r/tests/testthat` suite still
passes (1408 passed, 0 failed).
Minimal repro of bug 1 (pre-fix):
```r
library(nanoarrow)
schema <- na_list(na_int64())
arr <- as_nanoarrow_array(
list(bit64::as.integer64(c("9223372036854775295", "2")),
bit64::as.integer64("3")),
schema = schema
)
to <- vctrs::new_list_of(list(), ptype = bit64::integer64())
convert_array(arr, to = to)
#> [[1]]
#> integer64
#> [1] <NA> 0
#>
#> [[2]]
#> integer64
#> [1] 0
```
After this PR, this returns `list(c(9223372036854775295, 2), 3)` as expected.
Originally reported against a downstream package:
https://github.com/meztez/bigrquerystorage/issues/86.
--
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]
