From 0e4500f926d4332574db16f6c0bab87f8d3c79eb Mon Sep 17 00:00:00 2001
From: markfairbanks <mark.t.fairbanks@gmail.com>
Date: Thu, 28 Jul 2022 12:44:46 -0600
Subject: [PATCH 1/4] Use `setorder(na.last = TRUE)`

---
 R/step-subset-arrange.R | 1 +
 1 file changed, 1 insertion(+)

diff --git a/R/step-subset-arrange.R b/R/step-subset-arrange.R
index df1cfc01..0b475b45 100644
--- a/R/step-subset-arrange.R
+++ b/R/step-subset-arrange.R
@@ -28,6 +28,7 @@ arrange.dtplyr_step <- function(.data, ..., .by_group = FALSE) {
   # Order without grouping then restore
   dots <- set_names(dots, NULL)
   if (is_copied(.data) && no_transmute) {
+    dots <- c(dots, na.last = TRUE)
     step <- step_call(.data, "setorder", dots)
   } else {
     step <- step_subset(.data, i = call2("order", !!!dots), groups = character())

From 49a037ee06e9119edce38266073201ae3507c3ba Mon Sep 17 00:00:00 2001
From: markfairbanks <mark.t.fairbanks@gmail.com>
Date: Thu, 28 Jul 2022 12:45:30 -0600
Subject: [PATCH 2/4] Update tests

---
 tests/testthat/test-step-subset-arrange.R | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/test-step-subset-arrange.R b/tests/testthat/test-step-subset-arrange.R
index 2d9a2d44..457ba0cb 100644
--- a/tests/testthat/test-step-subset-arrange.R
+++ b/tests/testthat/test-step-subset-arrange.R
@@ -70,7 +70,7 @@ test_that("uses setorder when there is already a copy", {
 
   expect_equal(
     show_query(step_implicit),
-    expr(setorder(DT[x < 4], x, y))
+    expr(setorder(DT[x < 4], x, y, na.last = TRUE))
   )
 
   # Works with explicit copy
@@ -80,7 +80,7 @@ test_that("uses setorder when there is already a copy", {
 
   expect_equal(
     show_query(step_explicit),
-    expr(setorder(copy(DT)[, `:=`(x = x * 2)], x, -y))
+    expr(setorder(copy(DT)[, `:=`(x = x * 2)], x, -y, na.last = TRUE))
   )
 })
 

From 69c452857413e8fe5860457c8b282657ed0052a0 Mon Sep 17 00:00:00 2001
From: markfairbanks <mark.t.fairbanks@gmail.com>
Date: Thu, 28 Jul 2022 15:20:21 -0600
Subject: [PATCH 3/4] Add test for `NA` sorting

---
 tests/testthat/test-step-subset-arrange.R | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/testthat/test-step-subset-arrange.R b/tests/testthat/test-step-subset-arrange.R
index 457ba0cb..c86b0784 100644
--- a/tests/testthat/test-step-subset-arrange.R
+++ b/tests/testthat/test-step-subset-arrange.R
@@ -84,6 +84,18 @@ test_that("uses setorder when there is already a copy", {
   )
 })
 
+test_that("setorder places NAs last", {
+  dt <- lazy_dt(data.frame(x = c("b", NA, "a")), "DT")
+  dt$needs_copy <- TRUE
+
+  # Works with implicit copy
+  res <- dt %>%
+    arrange(x) %>%
+    as.data.table()
+
+  expect_equal(res$x, c("a", "b", NA))
+})
+
 test_that("works with a transmute expression", {
   dt <- lazy_dt(data.frame(x = 1:3, y = 1:3), "DT")
 

From 3693ab7582e4cc66ff5eefd9ac643fa6caf17070 Mon Sep 17 00:00:00 2001
From: markfairbanks <mark.t.fairbanks@gmail.com>
Date: Thu, 28 Jul 2022 15:29:14 -0600
Subject: [PATCH 4/4] Use a `tibble` in test

---
 tests/testthat/test-step-subset-arrange.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-step-subset-arrange.R b/tests/testthat/test-step-subset-arrange.R
index c86b0784..ca1d4579 100644
--- a/tests/testthat/test-step-subset-arrange.R
+++ b/tests/testthat/test-step-subset-arrange.R
@@ -85,7 +85,7 @@ test_that("uses setorder when there is already a copy", {
 })
 
 test_that("setorder places NAs last", {
-  dt <- lazy_dt(data.frame(x = c("b", NA, "a")), "DT")
+  dt <- lazy_dt(tibble(x = c("b", NA, "a")), "DT")
   dt$needs_copy <- TRUE
 
   # Works with implicit copy