You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2026/07/05 13:45:43 UTC
[PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
thisisnic opened a new pull request, #50374:
URL: https://github.com/apache/arrow/pull/50374
Still a draft as I need to work out what approach actually makes the most sense here
### Rationale for this change
### What changes are included in this PR?
### Are these changes tested?
### Are there any user-facing changes?
**This PR includes breaking changes to public APIs.** (If there are any breaking changes to public APIs, please explain which changes are breaking. If not, you can remove this.)
**This PR contains a "Critical Fix".** (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50374:
URL: https://github.com/apache/arrow/pull/50374#issuecomment-4886261644
:warning: GitHub issue #50339 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #50374:
URL: https://github.com/apache/arrow/pull/50374
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3551251624
##########
r/vignettes/data_types.Rmd:
##########
@@ -97,12 +97,15 @@ If the value in R does not fall within the permissible range for the correspondi
chunked_array(c(10L, 3L, 200L), type = int8())
```
-When translating from Arrow to R, integer types alway translate to R integers unless one of the following exceptions applies:
+When translating from Arrow to R, integer types always translate to R integers unless one of the following exceptions applies:
-- 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
+- If the value of an Arrow uint32 falls outside the range allowed for R integers, the result will be a numeric vector in R
- 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
- If the user sets `options(arrow.int64_downcast = FALSE)`, the Arrow int64 type always yields a `bit64::integer64` vector in R
regardless of the value
+ - Arrow uint64 types are always converted to numeric (double) vectors in R.
+ Note that doubles cannot exactly represent all uint64 values; precision may
+ be lost for values above 2^53.
Review Comment:
The new `uint64` bullet is indented as if it were a nested list item under the previous bullet, and the continuation lines are inconsistently indented. This will render oddly in the vignette (the `uint64` note may appear as a sub-bullet of the `arrow.int64_downcast` item).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3543021966
##########
r/man/acero.Rd:
##########
@@ -72,7 +72,7 @@ can assume that the function works in Acero just as it does in R.
Functions can be called either as \code{pkg::fun()} or just \code{fun()}, i.e. both
\code{str_sub()} and \code{stringr::str_sub()} work.
-In addition to these functions, you can call any of Arrow's 281 compute
+In addition to these functions, you can call any of Arrow's 253 compute
functions directly. Arrow has many functions that don't map to an existing R
Review Comment:
This hard-coded compute-function count is inconsistent with the roxygen source (`r/R/dplyr-funcs-doc.R` still says 281) and will likely drift again. Prefer removing the specific number (or updating it at the roxygen source and regenerating the .Rd) so the documentation stays consistent.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3542585592
##########
r/vignettes/data_types.Rmd:
##########
@@ -99,7 +99,8 @@ chunked_array(c(10L, 3L, 200L), type = int8())
When translating from Arrow to R, integer types alway translate to R integers unless one of the following exceptions applies:
Review Comment:
Typo: "alway" should be "always".
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3551246031
##########
r/R/type.R:
##########
@@ -356,13 +356,13 @@ NestedType <- R6Class("NestedType", inherit = DataType)
#' `date32()` creates a datetime type with a "day" unit, like the R `Date`
#' class. `date64()` has a "ms" unit.
#'
-#' `uint32` (32 bit unsigned integer), `uint64` (64 bit unsigned integer), and
-#' `int64` (64-bit signed integer) types may contain values that exceed the
-#' range of R's `integer` type (32-bit signed integer). When these arrow objects
-#' are translated to R objects, `uint32` and `uint64` are converted to `double`
-#' ("numeric") and `int64` is converted to `bit64::integer64`. For `int64`
-#' types, this conversion can be disabled (so that `int64` always yields a
-#' `bit64::integer64` object) by setting `options(arrow.int64_downcast =
+#' `uint64` (64 bit unsigned integer) is always converted to `double`
+#' ("numeric") in R. `uint32` (32 bit unsigned integer) and `int64` (64-bit
+#' signed integer) types may contain values that exceed the range of R's
+#' `integer` type (32-bit signed integer). When they do, `uint32` is converted
+#' to `double` ("numeric") and `int64` is converted to `bit64::integer64`. For
Review Comment:
I added something hopefully brief enough to be less verbose
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3546557990
##########
r/vignettes/data_types.Rmd:
##########
@@ -97,9 +97,10 @@ If the value in R does not fall within the permissible range for the correspondi
chunked_array(c(10L, 3L, 200L), type = int8())
```
-When translating from Arrow to R, integer types alway translate to R integers unless one of the following exceptions applies:
+When translating from Arrow to R, integer types always translate to R integers unless one of the following exceptions applies:
Review Comment:
❤️
##########
r/R/type.R:
##########
@@ -356,13 +356,13 @@ NestedType <- R6Class("NestedType", inherit = DataType)
#' `date32()` creates a datetime type with a "day" unit, like the R `Date`
#' class. `date64()` has a "ms" unit.
#'
-#' `uint32` (32 bit unsigned integer), `uint64` (64 bit unsigned integer), and
-#' `int64` (64-bit signed integer) types may contain values that exceed the
-#' range of R's `integer` type (32-bit signed integer). When these arrow objects
-#' are translated to R objects, `uint32` and `uint64` are converted to `double`
-#' ("numeric") and `int64` is converted to `bit64::integer64`. For `int64`
-#' types, this conversion can be disabled (so that `int64` always yields a
-#' `bit64::integer64` object) by setting `options(arrow.int64_downcast =
+#' `uint64` (64 bit unsigned integer) is always converted to `double`
+#' ("numeric") in R. `uint32` (32 bit unsigned integer) and `int64` (64-bit
+#' signed integer) types may contain values that exceed the range of R's
+#' `integer` type (32-bit signed integer). When they do, `uint32` is converted
+#' to `double` ("numeric") and `int64` is converted to `bit64::integer64`. For
Review Comment:
I agree with this comment, though it might make this a bit too verbose? But having some detail that there *is* a loss of precision here is good to call out
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #50374:
URL: https://github.com/apache/arrow/pull/50374#issuecomment-4950075206
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit ad4d8ac748fb4b99e70b235e138a6c2a9c50bc8a.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/86618645074) has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3544714217
##########
r/vignettes/data_types.Rmd:
##########
@@ -97,9 +97,10 @@ If the value in R does not fall within the permissible range for the correspondi
chunked_array(c(10L, 3L, 200L), type = int8())
```
-When translating from Arrow to R, integer types alway translate to R integers unless one of the following exceptions applies:
+When translating from Arrow to R, integer types always translate to R integers unless one of the following exceptions applies:
-- 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
+- Arrow uint64 types are always converted to numeric (double) vectors in R
Review Comment:
The new mapping “uint64 → R numeric (double)” is correct for type unification, but it can still silently lose integer precision once values exceed 2^53 (because R numeric is IEEE-754 double). It would help to mention this explicitly here so users don’t assume all uint64 values round-trip exactly.
##########
r/R/type.R:
##########
@@ -356,13 +356,13 @@ NestedType <- R6Class("NestedType", inherit = DataType)
#' `date32()` creates a datetime type with a "day" unit, like the R `Date`
#' class. `date64()` has a "ms" unit.
#'
-#' `uint32` (32 bit unsigned integer), `uint64` (64 bit unsigned integer), and
-#' `int64` (64-bit signed integer) types may contain values that exceed the
-#' range of R's `integer` type (32-bit signed integer). When these arrow objects
-#' are translated to R objects, `uint32` and `uint64` are converted to `double`
-#' ("numeric") and `int64` is converted to `bit64::integer64`. For `int64`
-#' types, this conversion can be disabled (so that `int64` always yields a
-#' `bit64::integer64` object) by setting `options(arrow.int64_downcast =
+#' `uint64` (64 bit unsigned integer) is always converted to `double`
+#' ("numeric") in R. `uint32` (32 bit unsigned integer) and `int64` (64-bit
+#' signed integer) types may contain values that exceed the range of R's
+#' `integer` type (32-bit signed integer). When they do, `uint32` is converted
+#' to `double` ("numeric") and `int64` is converted to `bit64::integer64`. For
Review Comment:
This documentation states that `uint64` is always converted to `double`, but it doesn’t mention that doubles can’t exactly represent all 64-bit integers (precision loss above 2^53). Adding that clarification would set expectations for users relying on large identifiers.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50339: [R] read_ipc_stream fails to unify nested uint64 fields inside a Struct array across record batches [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50374:
URL: https://github.com/apache/arrow/pull/50374#discussion_r3542605035
##########
r/tests/testthat/test-Array.R:
##########
@@ -521,23 +521,25 @@ test_that("Array<int8>$as_vector() converts to integer (ARROW-3794)", {
expect_as_vector(a, u8)
})
-test_that("Arrays of {,u}int{32,64} convert to integer if they can fit", {
+test_that("Arrays of {,u}int{32} convert to integer if they can fit", {
Review Comment:
The test description no longer matches what the test covers: it still asserts `int64` downcasting behavior but the name now only mentions `{,u}int{32}`. This can make future failures harder to interpret.
##########
r/man/acero.Rd:
##########
@@ -72,7 +72,7 @@ can assume that the function works in Acero just as it does in R.
Functions can be called either as \code{pkg::fun()} or just \code{fun()}, i.e. both
\code{str_sub()} and \code{stringr::str_sub()} work.
-In addition to these functions, you can call any of Arrow's 281 compute
+In addition to these functions, you can call any of Arrow's 253 compute
functions directly. Arrow has many functions that don't map to an existing R
Review Comment:
Hard-coding the number of available compute functions in the docs is brittle and already inconsistent with other package documentation (e.g., `r/R/dplyr-funcs-doc.R` still states 281). Since the count can change with Arrow versions/build options, prefer wording that avoids a specific number.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org