You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2023/03/07 18:05:44 UTC
[GitHub] [arrow] paleolimbot opened a new pull request, #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
paleolimbot opened a new pull request, #34489:
URL: https://github.com/apache/arrow/pull/34489
### Rationale for this change
When we attempt to re-use an object that Arrow itself created previously by wrapping a chunked array, we will get a crash if this object has been materialized (i.e., R values have been accessed and the ChunkedArray reference deleted). This behaviour changed between 10.0.0 and 11.0.0 because I redid the ALTREP implementation just after the 10.0.0 release.
The following test crashes R on main and 11.0.0 but passes after this PR:
``` r
library(arrow, warn.conflicts = FALSE)
#> Some features are not enabled in this build of Arrow. Run `arrow_info()` for more information.
library(testthat, warn.conflicts = FALSE)
withr::local_namespace("arrow")
test_that("Materialized ALTREP arrays don't cause arrow to crash when attempting to bypass", {
a_int <- Array$create(c(1L, 2L, 3L))
b_int <- a_int$as_vector()
expect_true(is_arrow_altrep(b_int))
expect_false(test_arrow_altrep_is_materialized(b_int))
# Some operations that use altrep bypass
expect_equal(infer_type(b_int), int32())
expect_equal(as_arrow_array(b_int), a_int)
# Still shouldn't have materialized yet
expect_false(test_arrow_altrep_is_materialized(b_int))
# Force it to materialize and check again
test_arrow_altrep_force_materialize(b_int)
expect_true(test_arrow_altrep_is_materialized(b_int))
expect_equal(infer_type(b_int), int32())
expect_equal(as_arrow_array(b_int), a_int)
})
#> Test passed 🎉
```
### What changes are included in this PR?
We used a function called `is_arrow_altrep()` to check if we could safely access the ChunkedArray reference; however, *materialized* ALTREP arrays still cause this return `true`. I added a new function `is_unmaterialized_arrow_altrep()` and replaced usage that depended on the ChunkedArray actually existing to use it.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34489:
URL: https://github.com/apache/arrow/pull/34489#issuecomment-1458608151
:warning: GitHub issue #34211 **has no components**, please add labels for components.
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34489:
URL: https://github.com/apache/arrow/pull/34489#issuecomment-1458608051
* Closes: #34211
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34489:
URL: https://github.com/apache/arrow/pull/34489#issuecomment-1458608121
:warning: GitHub issue #34211 **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
[GitHub] [arrow] thisisnic merged pull request #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #34489:
URL: https://github.com/apache/arrow/pull/34489
--
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
[GitHub] [arrow] ursabot commented on pull request #34489: GH-34211: [R] Make sure Arrow arrays are unmaterialized before attempting to access the underlying ChunkedArray
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34489:
URL: https://github.com/apache/arrow/pull/34489#issuecomment-1461071668
Benchmark runs are scheduled for baseline = fcc17e3561750249a2a7afc110cfa9c44c893022 and contender = 44b5d004000a33734b379cbb69349e32b7aa114c. 44b5d004000a33734b379cbb69349e32b7aa114c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b37ad8c43e044980a7cb1818a2b4daf4...a64e915b49924124b562de3f8ffdad0b/)
[Finished :arrow_down:0.79% :arrow_up:0.03%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/157cdf6687494e2a97eb6ea0c4190869...bd36312de576428d9e450e312552b200/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/34fd0abe09724d80b1d65a48922ccf10...3f2fe9e169da468f8595b7453516e19e/)
[Finished :arrow_down:0.35% :arrow_up:0.25%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6731e070de0b44b0b3e5ca0ae4ef9172...ca272353ef6d42019547999b3f8acf68/)
Buildkite builds:
[Finished] [`44b5d004` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2495)
[Finished] [`44b5d004` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2525)
[Finished] [`44b5d004` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2493)
[Finished] [`44b5d004` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2516)
[Finished] [`fcc17e35` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2494)
[Finished] [`fcc17e35` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2524)
[Finished] [`fcc17e35` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2492)
[Finished] [`fcc17e35` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2515)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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