Skip to content

Commit

Permalink
GH-42220: [R] handle vctrs_rcrd extension type in metadata cleaning (#…
Browse files Browse the repository at this point in the history
…42226)

### Rationale for this change

See #42220. Extension type metadata serialization works slightly differently from the general R metadata path: it uses a vctrs::ptype object, which is 0-length with attributes. The sanitization needs to happen on `attributes()`, not the object being serialized. 

The error was caused because `vctrs_rcrd` has a special `[<-` method, so iterating over it and assigning back in the sanitized version (even if it were the right thing to do) failed.

### What changes are included in this PR?

Sanitize the `attributes()` of vctrs ptypes.

### Are these changes tested?

yes

### Are there any user-facing changes?

other than the bug being fixed, no
* GitHub Issue: #42220

Authored-by: Neal Richardson <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
  • Loading branch information
nealrichardson authored Jun 22, 2024
1 parent c27c710 commit 230859b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
7 changes: 5 additions & 2 deletions r/R/extension.R
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ VctrsExtensionType <- R6Class("VctrsExtensionType",
paste0(capture.output(print(self$ptype())), collapse = "\n")
},
deserialize_instance = function() {
private$.ptype <- safe_r_metadata(safe_unserialize(self$extension_metadata()))
private$.ptype <- safe_unserialize(self$extension_metadata())
attributes(private$.ptype) <- safe_r_metadata(attributes(private$.ptype))
},
ExtensionEquals = function(other) {
inherits(other, "VctrsExtensionType") && identical(self$ptype(), other$ptype())
Expand Down Expand Up @@ -510,11 +511,13 @@ vctrs_extension_array <- function(x, ptype = vctrs::vec_ptype(x),
vctrs_extension_type <- function(x,
storage_type = infer_type(vctrs::vec_data(x))) {
ptype <- vctrs::vec_ptype(x)
# Make sure there are no unsupported objects buried in there
attributes(ptype) <- safe_r_metadata(attributes(ptype))

new_extension_type(
storage_type = storage_type,
extension_name = "arrow.r.vctrs",
extension_metadata = serialize(ptype, NULL),
extension_metadata = serialize(ptype, NULL, ascii = TRUE),
type_class = VctrsExtensionType
)
}
8 changes: 8 additions & 0 deletions r/tests/testthat/test-extension.R
Original file line number Diff line number Diff line change
Expand Up @@ -343,3 +343,11 @@ test_that("Dataset/arrow_dplyr_query can roundtrip extension types", {

expect_identical(unclass(roundtripped$extension), roundtripped$letter)
})

test_that("Handling vctrs_rcrd type", {
df <- data.frame(
x = vctrs::new_rcrd(fields = list(special = 1:3), class = "special")
)
tab <- arrow_table(df)
expect_identical(as.data.frame(tab), df)
})

0 comments on commit 230859b

Please sign in to comment.