Skip to content

Commit ad4d8ac

Browse files
thisisnicCopilot
andauthored
GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches (#50374)
### Rationale for this change Attempting downcasting on UINT64 Fields when converting them from Arrow to R works fine on individual arrays, but fails on list arrays, because we only use the individual row to work out whether to downcast. I considered a few different solutions but I concluded that this one is the least complex while presenting a good solution - having UINT64 always be converted to doubles and never attempt downcasting. The impact here is that people who are expecting integers via downcasting will see a change in behaviour, but I think there's a strong argument here that if you are using UINT64 to store values that can be downcast, you probably need to be using UINT32 anyway. Other options considered: fix this in `Converter_List` and check values to see if they can be downcast: significantly more complex and only fixes one level of nesting. I checked duckdb and they always convert UINT64 to doubles. ### What changes are included in this PR? Never try to downcast UNIT64 values when converting to R ### Are these changes tested? Yes ### Are there any user-facing changes? Breaking change as mentioned in rationale above. * GitHub Issue: #50339 Lead-authored-by: Nic Crane <thisisnic@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nic Crane <thisisnic@gmail.com>
1 parent 934fa87 commit ad4d8ac

7 files changed

Lines changed: 73 additions & 57 deletions

File tree

r/NEWS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919

2020
# arrow 24.0.0.9000
2121

22-
# arrow 24.0.0
22+
- Arrow `uint64` types are now always converted to R `double` (numeric) vectors,
23+
regardless of the values. Previously, small `uint64` values were converted to
24+
R `integer`, which could cause inconsistent types within list columns when
25+
different list elements had different value ranges (#50339).
2326

2427
# arrow 24.0.0
2528

r/R/type.R

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -356,13 +356,14 @@ NestedType <- R6Class("NestedType", inherit = DataType)
356356
#' `date32()` creates a datetime type with a "day" unit, like the R `Date`
357357
#' class. `date64()` has a "ms" unit.
358358
#'
359-
#' `uint32` (32 bit unsigned integer), `uint64` (64 bit unsigned integer), and
360-
#' `int64` (64-bit signed integer) types may contain values that exceed the
361-
#' range of R's `integer` type (32-bit signed integer). When these arrow objects
362-
#' are translated to R objects, `uint32` and `uint64` are converted to `double`
363-
#' ("numeric") and `int64` is converted to `bit64::integer64`. For `int64`
364-
#' types, this conversion can be disabled (so that `int64` always yields a
365-
#' `bit64::integer64` object) by setting `options(arrow.int64_downcast =
359+
#' `uint64` (64 bit unsigned integer) is always converted to `double`
360+
#' ("numeric") in R. Note that doubles cannot exactly represent all uint64
361+
#' values; precision may be lost for values above 2^53. `uint32` (32 bit unsigned integer) and `int64` (64-bit
362+
#' signed integer) types may contain values that exceed the range of R's
363+
#' `integer` type (32-bit signed integer). When they do, `uint32` is converted
364+
#' to `double` ("numeric") and `int64` is converted to `bit64::integer64`. For
365+
#' `int64` types, this conversion can be disabled (so that `int64` always yields
366+
#' a `bit64::integer64` object) by setting `options(arrow.int64_downcast =
366367
#' FALSE)`.
367368
#'
368369
#' `decimal128()` creates a `Decimal128Type`. Arrow decimals are fixed-point

r/man/data-type.Rd

Lines changed: 8 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

r/src/array_to_vector.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,13 +1296,8 @@ std::shared_ptr<Converter> Converter::Make(
12961296
}
12971297

12981298
case Type::UINT64:
1299-
if (ArraysCanFitInteger(chunked_array->chunks())) {
1300-
return std::make_shared<arrow::r::Converter_Int<arrow::UInt64Type>>(
1301-
chunked_array);
1302-
} else {
1303-
return std::make_shared<arrow::r::Converter_Double<arrow::UInt64Type>>(
1304-
chunked_array);
1305-
}
1299+
return std::make_shared<arrow::r::Converter_Double<arrow::UInt64Type>>(
1300+
chunked_array);
13061301

13071302
case Type::HALF_FLOAT:
13081303
return std::make_shared<arrow::r::Converter_Double<arrow::HalfFloatType>>(

r/src/arrowExports.cpp

Lines changed: 22 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

r/tests/testthat/test-Array.R

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -521,23 +521,25 @@ test_that("Array<int8>$as_vector() converts to integer (ARROW-3794)", {
521521
expect_as_vector(a, u8)
522522
})
523523

524-
test_that("Arrays of {,u}int{32,64} convert to integer if they can fit", {
524+
test_that("Arrays of uint32 and int64 convert to integer if they can fit", {
525525
u32 <- arrow_array(1L)$cast(uint32())
526526
expect_identical(as.vector(u32), 1L)
527527

528-
u64 <- arrow_array(1L)$cast(uint64())
529-
expect_identical(as.vector(u64), 1L)
530-
531528
i64 <- arrow_array(bit64::as.integer64(1:10))
532529
expect_identical(as.vector(i64), 1:10)
533530
})
534531

535-
test_that("Arrays of uint{32,64} convert to numeric if they can't fit integer", {
532+
test_that("Arrays of uint32 convert to numeric if they can't fit integer", {
536533
u32 <- arrow_array(bit64::as.integer64(1) + MAX_INT)$cast(uint32())
537534
expect_identical(as.vector(u32), 1 + MAX_INT)
535+
})
538536

539-
u64 <- arrow_array(bit64::as.integer64(1) + MAX_INT)$cast(uint64())
540-
expect_identical(as.vector(u64), 1 + MAX_INT)
537+
test_that("Arrays of uint64 always convert to numeric (double)", {
538+
u64_small <- arrow_array(1L)$cast(uint64())
539+
expect_identical(as.vector(u64_small), 1)
540+
541+
u64_large <- arrow_array(bit64::as.integer64(1) + MAX_INT)$cast(uint64())
542+
expect_identical(as.vector(u64_large), 1 + MAX_INT)
541543
})
542544

543545
test_that("arrow_array() recognise arrow::Array (ARROW-3815)", {
@@ -1453,3 +1455,14 @@ test_that("Array handles negative fractional dates correctly (GH-46873)", {
14531455
arr <- arrow_array(d)
14541456
expect_equal(as.vector(arr), as.Date("1969-12-31", origin = "1970-01-01"))
14551457
})
1458+
1459+
test_that("uint64 inside list columns always converts to double (GH-50339)", {
1460+
list_arr <- arrow_array(
1461+
list(1, 9999999999),
1462+
type = list_of(uint64())
1463+
)
1464+
1465+
result <- as.vector(list_arr)
1466+
expect_type(result[[1]], "double")
1467+
expect_type(result[[2]], "double")
1468+
})

r/vignettes/data_types.Rmd

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,15 @@ If the value in R does not fall within the permissible range for the correspondi
9797
chunked_array(c(10L, 3L, 200L), type = int8())
9898
```
9999

100-
When translating from Arrow to R, integer types alway translate to R integers unless one of the following exceptions applies:
100+
When translating from Arrow to R, integer types always translate to R integers unless one of the following exceptions applies:
101101

102-
- If the value of an Arrow uint32 or uint64 falls outside the range allowed for R integers, the result will be a numeric vector in R
102+
- If the value of an Arrow uint32 falls outside the range allowed for R integers, the result will be a numeric vector in R
103103
- If the value of an Arrow int64 variable falls outside the range allowed for R integers, the result will be a `bit64::integer64` vector in R
104104
- If the user sets `options(arrow.int64_downcast = FALSE)`, the Arrow int64 type always yields a `bit64::integer64` vector in R
105105
regardless of the value
106+
- Arrow uint64 types are always converted to numeric (double) vectors in R.
107+
Note that doubles cannot exactly represent all uint64 values; precision may
108+
be lost for values above 2^53.
106109

107110
## Floating point numeric types
108111

@@ -351,7 +354,7 @@ to Arrow list type (which is a "list of" some type).
351354
| uint8 | integer |
352355
| uint16 | integer |
353356
| uint32 | integer ^1^ |
354-
| uint64 | integer ^1^ |
357+
| uint64 | double |
355358
| float16 | - ^2^ |
356359
| float32 | double |
357360
| float64 | double |
@@ -376,9 +379,9 @@ to Arrow list type (which is a "list of" some type).
376379
| map | arrow_list ^5^ |
377380
| union | - ^2^ |
378381

379-
^1^: These integer types may contain values that exceed the range of R's
380-
`integer` type (32 bit signed integer). When they do, `uint32` and `uint64` are
381-
converted to `double` ("numeric") and `int64` is converted to
382+
^1^: These integer types may contain values that exceed the range of R's
383+
`integer` type (32 bit signed integer). When they do, `uint32` is
384+
converted to `double` ("numeric") and `int64` is converted to
382385
`bit64::integer64`. This conversion can be disabled (so that `int64` always
383386
yields a `bit64::integer64` vector) by setting `options(arrow.int64_downcast = FALSE)`.
384387

0 commit comments

Comments
 (0)