window_order crashing when deleting the variable that I am ordering for

[catalamarti]

Before when using window_order I could do something for example:

x <- table_x %>% window_order(variable1) %>% .... %>% select(-variable1)

Now when the variable that you ordered for is not present it crashes.

Example with dbplyr 2.0.0:

Now with the new version dbplyr 2.3.0:

As far as I know there is no way to delete the ordering so I am stuck and I have to use the previous version.
Is this something on purpose? Then how we can solve this error?

[mgirlich]

It isn't intentional but it kind of worked accidentally before.

@catalamarti We'll try to fix this soon. As a workaround you could remove the window order by using an empty window_order() after your mutate() step.

A reprex

library(dplyr, warn.conflicts = FALSE)
library(dbplyr, warn.conflicts = FALSE)

memdb_frame(x = 1, y = 1) %>% 
  window_order(y) %>% 
  select(-y) %>% 
  compute()
#> Error in `window_order()`:
#> ℹ In argument: `y`
#> Caused by error:
#> ! Object `y` not found.

#> Backtrace:
#>      ▆
#>   1. ├─... %>% compute()
#>   2. ├─dplyr::compute(.)
#>   3. ├─dbplyr:::compute.tbl_sql(.)
#>   4. │ └─... %>% window_order(!!!op_sort(x))
#>   5. └─dbplyr::window_order(., !!!op_sort(x))
#>   6.   └─dbplyr:::partial_eval_dots(.data, ..., .named = FALSE)
#>   7.     └─dbplyr:::partial_eval_quo(dot, .data, error_call, dot_name, was_named[[i]])
#>   8.       ├─rlang::try_fetch(...)
#>   9.       │ ├─base::tryCatch(...)
#>  10.       │ │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  11.       │ │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  12.       │ │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>  13.       │ └─base::withCallingHandlers(...)
#>  14.       └─dbplyr::partial_eval(get_expr(x), data, get_env(x), error_call = error_call)
#>  15.         └─dbplyr:::partial_eval_sym(call, data, env)
#>  16.           └─cli::cli_abort("Object {.var {name}} not found.", call = NULL)
#>  17.             └─rlang::abort(...)

^{Created on 2023-01-19 with reprex v2.0.2}

@hadley The issue is that select() removes a variable that is used in window_order() but the order property is kept as is. This is similar to removing a grouping variable but select() has a special handling for this.

I would propose:

1. Disallow expressions in window_order(). Currently, it is possible to do something like window_order(y + x). This is probably barely ever useful and causes more difficulties for the next step.
   This would be a breaking change but a small one I guess.
2. Update the window order attribute when removing a column used in window_order(). That is:

library(dplyr, warn.conflicts = FALSE)
library(dbplyr, warn.conflicts = FALSE)

lf <- lazy_frame(x = 1, y = 1) %>% 
  window_order(y) %>% 
  select(-y)
# CRAN
lf$lazy_query$order_vars
#> [[1]]
#> <quosure>
#> expr: ^y
#> env:  0x7fd5e6bb78f8

# This should rather be empty

This would also allow to simplify order_vars from a list of quosures to a character vector like group_vars.
What do you think?

[catalamarti]

Using empty window_order() worked fine. It would be nice if it is fixed but as far as there is a solution it is fine.
Thank you very much!

[hadley]

@mgirlich that proposal sounds good to me.

[mgirlich]

@catalamarti The PR should fix your issue. If you want to try it out install via devtools::install_github("tidyverse/dbplyr#1106").