This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new 02824cc4 fix(r): correct two int64 corruption bugs affecting
list<int64> conversion (#933)
02824cc4 is described below
commit 02824cc4149ce1f7e80e25406da81ef76efbe99a
Author: Bruno Tremblay <[email protected]>
AuthorDate: Fri Sep 4 13:36:58 2026 -0400
fix(r): correct two int64 corruption bugs affecting list<int64> conversion
(#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.
---
r/R/as-array.R | 2 +-
r/src/materialize_int64.h | 2 +-
r/tests/testthat/test-as-array.R | 23 +++++++++++++++++++++++
r/tests/testthat/test-convert-array.R | 18 ++++++++++++++++++
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/r/R/as-array.R b/r/R/as-array.R
index d3aa5ee9..c45fdf24 100644
--- a/r/R/as-array.R
+++ b/r/R/as-array.R
@@ -257,7 +257,7 @@ as_nanoarrow_array.list <- function(x, ..., schema = NULL) {
array <- nanoarrow_array_init(schema)
- child <- unlist(x, recursive = FALSE, use.names = FALSE)
+ child <- do.call(c, x)
if (is.null(child)) {
child_array <- as_nanoarrow_array.vctrs_unspecified(logical(), schema =
na_na())
} else {
diff --git a/r/src/materialize_int64.h b/r/src/materialize_int64.h
index ad83671e..086a7505 100644
--- a/r/src/materialize_int64.h
+++ b/r/src/materialize_int64.h
@@ -49,7 +49,7 @@ static inline int nanoarrow_materialize_int64(struct
ArrayViewSlice* src,
break;
case NANOARROW_TYPE_INT64:
memcpy(result + dst->offset,
- src->array_view->buffer_views[1].data.as_int32 + raw_src_offset,
+ src->array_view->buffer_views[1].data.as_int64 + raw_src_offset,
dst->length * sizeof(int64_t));
// Set any nulls to NA_INTEGER64
diff --git a/r/tests/testthat/test-as-array.R b/r/tests/testthat/test-as-array.R
index 6157bf55..7c301e0b 100644
--- a/r/tests/testthat/test-as-array.R
+++ b/r/tests/testthat/test-as-array.R
@@ -824,6 +824,29 @@ test_that("as_nanoarrow_array() works for list(integer())
-> na_list(na_int32())
expect_identical(array$children[[1]]$length, 10L)
})
+test_that("as_nanoarrow_array() works for list(integer64()) ->
na_list(na_int64())", {
+ # GH932: unlist() silently strips the integer64 class from list elements,
+ # reinterpreting the underlying int64 bit pattern as a double and corrupting
+ # values before the child array is even built.
+ skip_if_not_installed("bit64")
+
+ big <- bit64::as.integer64(c("9223372036854775295", "2"))
+ x <- list(big, bit64::as.integer64("3"))
+ array <- as_nanoarrow_array(x, schema = na_list(na_int64()))
+
+ expect_identical(infer_nanoarrow_schema(array)$format, "+l")
+ expect_identical(array$length, 2L)
+ expect_identical(array$null_count, 0L)
+ expect_identical(infer_nanoarrow_schema(array$children[[1]])$format, "l")
+ expect_identical(array$children[[1]]$length, 3L)
+
+ to <- vctrs::new_list_of(list(), ptype = bit64::integer64())
+ expect_identical(
+ convert_array(array, to),
+ vctrs::new_list_of(list(big, bit64::as.integer64("3")), ptype =
bit64::integer64())
+ )
+})
+
test_that("as_nanoarrow_array() works for unspecified() -> na_na()", {
skip_if_not_installed("vctrs")
diff --git a/r/tests/testthat/test-convert-array.R
b/r/tests/testthat/test-convert-array.R
index 8ceca779..deedb7e3 100644
--- a/r/tests/testthat/test-convert-array.R
+++ b/r/tests/testthat/test-convert-array.R
@@ -744,6 +744,24 @@ test_that("convert to vector works for valid integer64()",
{
)
})
+test_that("convert to vector works for int64 with a nonzero offset", {
+ # GH932: the int64 materializer read from buffer_views[1].data.as_int32
+ # (i.e., with int32-sized pointer arithmetic) instead of as_int64, so any
+ # slice with a nonzero starting offset (e.g., a list child sliced per-row)
+ # read from the wrong memory location.
+ skip_if_not_installed("bit64")
+
+ vals <- bit64::as.integer64(c("9223372036854775295", "2", "3"))
+ array <- as_nanoarrow_array(vals, schema = na_int64())
+
+ sliced <- nanoarrow_array_modify(array, list(offset = 1L, length = 2L))
+
+ expect_identical(
+ convert_array(sliced, bit64::integer64()),
+ vals[2:3]
+ )
+})
+
test_that("convert to vector works for null -> integer64()", {
skip_if_not_installed("bit64")
