Skip to content

Commit

Permalink
Use different sequences for tables and columns
Browse files Browse the repository at this point in the history
  • Loading branch information
mgirlich committed Jun 13, 2023
1 parent c56e61a commit 3f4bfe4
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 26 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# dbplyr (development version)

* The columns generated when using a window function in `filter()` are now named
`col01` etc. instead of `q01()` (@mgirlich, #1258).

* `slice_*()` now supports the data masking pronouns `.env` and `.data` (@mgirlich, #1294).

* `tbl()` now informs when the user probably forgot to wrap the table identifier
Expand Down
1 change: 1 addition & 0 deletions R/sql-build.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#' @param ... Other arguments passed on to the methods. Not currently used.
sql_build <- function(op, con = NULL, ..., use_star = TRUE) {
unique_subquery_name_reset()
unique_column_name_reset()
check_dots_used()
UseMethod("sql_build")
}
Expand Down
2 changes: 1 addition & 1 deletion R/translate-sql-window.R
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ translate_window_where <- function(expr, window_funs = common_window_funs()) {
if (is_formula(expr)) {
translate_window_where(f_rhs(expr), window_funs)
} else if (is_call(expr, name = window_funs)) {
name <- unique_subquery_name()
name <- unique_column_name()
window_where(sym(name), set_names(list(expr), name))
} else {
args <- lapply(expr[-1], translate_window_where, window_funs = window_funs)
Expand Down
9 changes: 9 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,18 @@ unique_subquery_name <- function() {
options(dbplyr_subquery_name = i)
sprintf("q%02i", i)
}
unique_column_name <- function() {
# Needs to use option so can reset at the start of each query
i <- getOption("dbplyr_column_name", 0) + 1
options(dbplyr_column_name = i)
sprintf("col%02i", i)
}
unique_subquery_name_reset <- function() {
options(dbplyr_subquery_name = 0)
}
unique_column_name_reset <- function() {
options(dbplyr_column_name = 0)
}

succeeds <- function(x, quiet = FALSE) {
tryCatch(
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/backend-mssql.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,10 @@
<SQL>
SELECT `x`
FROM (
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY RAND()) AS `q01`
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY RAND()) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)

---

Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/backend-mysql.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@
<SQL>
SELECT `x`
FROM (
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY RAND()) AS `q01`
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY RAND()) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)

---

Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/backend-oracle.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@
<SQL>
SELECT `x`
FROM (
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY DBMS_RANDOM.VALUE()) AS `q01`
SELECT `df`.*, ROW_NUMBER() OVER (ORDER BY DBMS_RANDOM.VALUE()) AS `col01`
FROM `df`
) `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)

# copy_inline uses UNION ALL

Expand Down
6 changes: 4 additions & 2 deletions tests/testthat/_snaps/verb-distinct.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
<SQL>
SELECT `x`, `y`
FROM (
SELECT `df`.*, ROW_NUMBER() OVER (PARTITION BY `x` ORDER BY `y` DESC) AS `q01`
SELECT
`df`.*,
ROW_NUMBER() OVER (PARTITION BY `x` ORDER BY `y` DESC) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` = 1)
WHERE (`col01` = 1)

12 changes: 6 additions & 6 deletions tests/testthat/_snaps/verb-filter.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
Output
<SQL> SELECT `x`, `y`
FROM (
SELECT `df`.*, AVG(`x`) OVER () AS `q01`
SELECT `df`.*, AVG(`x`) OVER () AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` > 3.0) AND (`y` < 3.0)
WHERE (`col01` > 3.0) AND (`y` < 3.0)

# errors for named input

Expand Down Expand Up @@ -66,10 +66,10 @@
<SQL>
SELECT `x`
FROM (
SELECT `df`.*, MAX(`x`) OVER () AS `q01`
SELECT `df`.*, MAX(`x`) OVER () AS `col01`
FROM `df`
) AS `q01`
WHERE (`x` = `q01`) AND (`x` IN (1, 2))
WHERE (`x` = `col01`) AND (`x` IN (1, 2))

# filter() after summarise() uses `HAVING`

Expand Down Expand Up @@ -152,12 +152,12 @@
FROM (
SELECT
`q01`.*,
SUM(`x_mean`) OVER (PARTITION BY `g` ROWS UNBOUNDED PRECEDING) AS `q01`
SUM(`x_mean`) OVER (PARTITION BY `g` ROWS UNBOUNDED PRECEDING) AS `col01`
FROM (
SELECT `g`, `h`, AVG(`x`) AS `x_mean`
FROM `df`
GROUP BY `g`, `h`
) AS `q01`
) AS `q01`
WHERE (`q01` = 1.0)
WHERE (`col01` = 1.0)

12 changes: 6 additions & 6 deletions tests/testthat/_snaps/verb-slice.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,30 +74,30 @@
<SQL>
SELECT `x`, `id`
FROM (
SELECT `df`.*, RANK() OVER (ORDER BY `x` DESC) AS `q01`
SELECT `df`.*, RANK() OVER (ORDER BY `x` DESC) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)
Code
lf %>% slice_max(.data$x)
Output
<SQL>
SELECT `x`, `id`
FROM (
SELECT `df`.*, RANK() OVER (ORDER BY `x` DESC) AS `q01`
SELECT `df`.*, RANK() OVER (ORDER BY `x` DESC) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)
Code
lf %>% slice_max(.data$x * .env$x)
Output
<SQL>
SELECT `x`, `id`
FROM (
SELECT `df`.*, RANK() OVER (ORDER BY `x` * -1 DESC) AS `q01`
SELECT `df`.*, RANK() OVER (ORDER BY `x` * -1 DESC) AS `col01`
FROM `df`
) AS `q01`
WHERE (`q01` <= 1)
WHERE (`col01` <= 1)

# slice_sample errors when expected

Expand Down
9 changes: 4 additions & 5 deletions tests/testthat/test-verb-filter.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ test_that("two filters equivalent to one", {
expect_equal(lf1 %>% remote_query(), lf2 %>% remote_query())
expect_snapshot(lf1 %>% remote_query())

unique_subquery_name_reset()
df1 <- mf %>% filter(mean(x, na.rm = TRUE) > 3) %>% filter(y < 3)
unique_subquery_name_reset()
df2 <- mf %>% filter(mean(x, na.rm = TRUE) > 3, y < 3)
compare_tbl(df1, df2)

unique_column_name_reset()
lf1 <- lf %>% filter(mean(x, na.rm = TRUE) > 3) %>% filter(y < 3)
unique_subquery_name_reset()
unique_column_name_reset()
lf2 <- lf %>% filter(mean(x, na.rm = TRUE) > 3, y < 3)
expect_equal(lf1 %>% remote_query(), lf2 %>% remote_query())
expect_snapshot(lf1 %>% remote_query())
Expand Down Expand Up @@ -380,7 +379,7 @@ test_that("generates correct lazy_select_query", {
lazy_select_query(
x = out$x,
select = syms(set_names(colnames(lf))),
where = list(expr(q01 > 1))
where = list(expr(col01 > 1))
),
ignore_formula_env = TRUE
)
Expand All @@ -390,7 +389,7 @@ test_that("generates correct lazy_select_query", {
lazy_select_query(
x = lf$lazy_query,
select_operation = "mutate",
select = list(x = sym("x"), y = sym("y"), q01 = quo(mean(x, na.rm = TRUE)))
select = list(x = sym("x"), y = sym("y"), col01 = quo(mean(x, na.rm = TRUE)))
),
ignore_formula_env = TRUE
)
Expand Down

0 comments on commit 3f4bfe4

Please sign in to comment.