From fe6c067e690ab28392d19388810725ce9f7ad646 Mon Sep 17 00:00:00 2001
From: Dewey Dunnington <dewey@voltrondata.com>
Date: Tue, 6 Jun 2023 13:36:43 -0400
Subject: [PATCH] GH-35952: [R] Ensure that schema metadata can actually be set
 as a named character vector (#35954)

This wasn't necessarily a regression (reprex fails in 12.0.0 as well), although the comments suggest that assigning a named character vector will work when assigning schema metadata and this feature appears to be used by at least one of our dependencies (sfarrow). Given that the sfarrow check passes on 12.0.0, there is possibly also a place in our code that returns a named character vector rather than a list. I've confirmed that this fix solves the reverse dependency failure building the sfarrow example vignette.

``` r
library(arrow, warn.conflicts = FALSE)

schema <- schema(x = int32())
schema$metadata <- c("name" = "value")
schema$metadata
#> $name
#> [1] "value"
```

<sup>Created on 2023-06-06 with [reprex v2.0.2](https://reprex.tidyverse.org)</sup>
* Closes: #35952

Authored-by: Dewey Dunnington <dewey@voltrondata.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
---
 r/R/schema.R                   |  4 ++++
 r/tests/testthat/test-schema.R | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/r/R/schema.R b/r/R/schema.R
index 9ff38487c4b6f..dc0b4ba81fbbc 100644
--- a/r/R/schema.R
+++ b/r/R/schema.R
@@ -229,9 +229,13 @@ prepare_key_value_metadata <- function(metadata) {
       call. = FALSE
     )
   }
+
+  metadata <- as.list(metadata)
+
   if (!is_empty(metadata) && is.list(metadata[["r"]])) {
     metadata[["r"]] <- .serialize_arrow_r_metadata(metadata[["r"]])
   }
+
   map_chr(metadata, as.character)
 }
 
diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R
index 24776e6d0c199..a6a0555eaca0e 100644
--- a/r/tests/testthat/test-schema.R
+++ b/r/tests/testthat/test-schema.R
@@ -140,6 +140,22 @@ test_that("Schema modification", {
   expect_error(schm[[c(2, 4)]] <- int32(), "length(i) not equal to 1", fixed = TRUE)
 })
 
+test_that("Metadata can be reassigned as a whole", {
+  schm <- schema(b = double(), c = string(), d = int8())
+
+  # Check named character vector
+  schm$metadata <- c("foo" = "bar")
+  expect_identical(schm$metadata, list(foo = "bar"))
+
+  # Check list()
+  schm$metadata <- list("foo" = "bar")
+  expect_identical(schm$metadata, list(foo = "bar"))
+
+  # Check NULL for removal
+  schm$metadata <- NULL
+  expect_identical(schm$metadata, set_names(list(), character()))
+})
+
 test_that("Metadata is preserved when modifying Schema", {
   schm <- schema(b = double(), c = string(), d = int8())
   schm$metadata$foo <- "bar"