From 4714a47b1b12b317066eb65d1d688abf5c64ecb3 Mon Sep 17 00:00:00 2001
From: Danny Smith <danny@gorcha.org>
Date: Thu, 1 Feb 2024 16:34:09 +1100
Subject: [PATCH] Fix bug in `rows_patch.tbl_lazy()` throwing an error when
 multiple columns need to be patched (#1444)

* Fix bug in `rows_patch.tbl_lazy()` throwing an error when multiple columns need to be patched
---
 NEWS.md                       |  3 +++
 R/rows.R                      |  6 +++++-
 tests/testthat/_snaps/rows.md | 25 +++++++++++++++++++++++++
 tests/testthat/test-rows.R    | 26 ++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/NEWS.md b/NEWS.md
index c5a3c5fc5..54b4b0347 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,8 @@
 # dbplyr (development version)
 
+* `rows_patch(in_place = FALSE)` now works when more than one column should be
+  patched (@gorcha, #1443).
+
 * Namespaced dplyr calls now error if the function doesn't exist, or 
   a translation is not available (#1426).
 
diff --git a/R/rows.R b/R/rows.R
index 7aa18cb71..4c014ea44 100644
--- a/R/rows.R
+++ b/R/rows.R
@@ -332,7 +332,11 @@ rows_patch.tbl_lazy <- function(x,
     )
 
     patch_columns_y <- paste0(new_columns, "...y")
-    patch_quos <- lapply(new_columns, function(.x) quo(coalesce(!!sym(.x), !!sym(patch_columns_y)))) %>%
+    patch_quos <-
+      lapply(
+        seq_along(new_columns),
+        function(.x) quo(coalesce(!!sym(new_columns[.x]), !!sym(patch_columns_y[.x])))
+      ) %>%
       rlang::set_names(new_columns)
     if (is_empty(new_columns)) {
       patched <- to_patch
diff --git a/tests/testthat/_snaps/rows.md b/tests/testthat/_snaps/rows.md
index 0f9229fc9..3abb7b43a 100644
--- a/tests/testthat/_snaps/rows.md
+++ b/tests/testthat/_snaps/rows.md
@@ -361,6 +361,31 @@
           ON (`df_x`.`x` = `df_y`.`x`)
       ) AS `q01`
 
+# `rows_patch()` works with multiple columns to update
+
+    Code
+      rows_patch(lazy_frame(x = 1:3, y = c(11, 12, NA), z = c(31, NA, 33), .name = "df_x"),
+      lazy_frame(x = 2:3, y = 22:23, z = 42:43, .name = "df_y"), by = "x", unmatched = "ignore",
+      in_place = FALSE)
+    Output
+      <SQL>
+      SELECT `df_x`.*
+      FROM `df_x`
+      WHERE NOT EXISTS (
+        SELECT 1 FROM `df_y`
+        WHERE (`df_x`.`x` = `df_y`.`x`)
+      )
+      
+      UNION ALL
+      
+      SELECT `x`, COALESCE(`y`, `y...y`) AS `y`, COALESCE(`z`, `z...y`) AS `z`
+      FROM (
+        SELECT `df_x`.*, `df_y`.`y` AS `y...y`, `df_y`.`z` AS `z...y`
+        FROM `df_x`
+        INNER JOIN `df_y`
+          ON (`df_x`.`x` = `df_y`.`x`)
+      ) AS `q01`
+
 # `rows_patch()` works with `in_place = TRUE`
 
     Code
diff --git a/tests/testthat/test-rows.R b/tests/testthat/test-rows.R
index 793606a90..59596bb86 100644
--- a/tests/testthat/test-rows.R
+++ b/tests/testthat/test-rows.R
@@ -612,6 +612,32 @@ test_that("`rows_patch()` works with `in_place = FALSE`", {
   expect_equal(df %>% collect(), tibble(x = 1:3, y = c(11, 12, NA)))
 })
 
+test_that("`rows_patch()` works with multiple columns to update", {
+  expect_snapshot(
+    rows_patch(
+      lazy_frame(x = 1:3, y = c(11, 12, NA), z = c(31, NA, 33), .name = "df_x"),
+      lazy_frame(x = 2:3, y = 22:23, z = 42:43, .name = "df_y"),
+      by = "x",
+      unmatched = "ignore",
+      in_place = FALSE
+    )
+  )
+
+  df <- memdb_frame(x = 1:3, y = c(11, 12, NA), z = c(31, NA, 33))
+  expect_equal(
+    rows_patch(
+      df, memdb_frame(x = 2:3, y = 22:23, z = 42:43),
+      by = "x",
+      unmatched = "ignore",
+      in_place = FALSE
+    ) %>%
+      collect(),
+    tibble(x = 1:3, y = c(11, 12, 23), z = c(31, 42, 33))
+  )
+
+  expect_equal(df %>% collect(), tibble(x = 1:3, y = c(11, 12, NA), z = c(31, NA, 33)))
+})
+
 test_that("`rows_patch()` works with `in_place = FALSE` and `returning`", {
   expect_equal(
     rows_patch(