ARROW-13125: [R] Throw error when 2+ args passed to desc() in arrange()

This throws an error if the user passes two or more arguments to `desc()` in `arrange()`. Previously the second argument was silently ignored. The zero-arguments case is already handled. Closes apache#10559 from ianmcook/ARROW-13125 Authored-by: Ian Cook <[email protected]> Signed-off-by: Ian Cook <[email protected]>
davidhao3300 · Jun 25, 2021 · dedcbc0 · dedcbc0
1 parent c4a20e9
commit dedcbc0
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 1 deletion.
diff --git a/r/R/dplyr-arrange.R b/r/R/dplyr-arrange.R
@@ -77,6 +77,11 @@ find_and_remove_desc <- function(quosure) {
       # remove enclosing parentheses
       expr <- expr[[2]]
     } else if (identical(expr[[1]], quote(desc))) {
+      # ensure desc() has only one argument (when an R expression is a function
+      # call, length == 2 means it has exactly one argument)
+      if (length(expr) > 2) {
+        stop("desc() expects only one argument", call. = FALSE)
+      }
       # remove desc() and toggle descending
       expr <- expr[[2]]
       descending <- !descending
@@ -90,4 +95,4 @@ find_and_remove_desc <- function(quosure) {
       desc = descending
     )
   )
-}
+}
diff --git a/r/tests/testthat/test-dplyr-arrange.R b/r/tests/testthat/test-dplyr-arrange.R
@@ -197,4 +197,11 @@ test_that("arrange() with bad inputs", {
     "not found",
     fixed = TRUE
   )
+  expect_error(
+    tbl %>%
+      Table$create() %>%
+      arrange(desc(int, chr)),
+    "expects only one argument",
+    fixed = TRUE
+  )
 })