Skip to content

Commit

Permalink
unique_subquery_name() no longer needs con
Browse files Browse the repository at this point in the history
  • Loading branch information
hadley committed Dec 1, 2023
1 parent 12f682f commit bff0130
Show file tree
Hide file tree
Showing 10 changed files with 17 additions and 13 deletions.
2 changes: 1 addition & 1 deletion R/backend-teradata.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ sql_query_select.Teradata <- function(con,
lvl = lvl + 1
)

alias <- unique_subquery_name(con)
alias <- unique_subquery_name()
from <- sql_query_wrap(con, unlimited_query, name = alias)
select_outer <- sql_star(con, alias)
out <- sql_select_clauses(con,
Expand Down
6 changes: 3 additions & 3 deletions R/db-sql.R
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ sql_query_wrap.DBIConnection <- function(con, from, name = NULL, ..., lvl = 0) {

from <- sql_indent_subquery(from, con, lvl)
# some backends, e.g. Postgres, require an alias for a subquery
name <- name %||% unique_subquery_name(con)
glue_sql2(con, "{from}", as_sql, "{.name name}")
name <- name %||% unique_subquery_name()
glue_sql2(con, "{from}", as_sql, "{.tbl name}")
} else { # must be a table_name
if (!is.null(name)) {
table <- db_table_name_extract(con, name)
Expand Down Expand Up @@ -1172,7 +1172,7 @@ dbplyr_sql_subquery <- function(con, ...) {
#' @importFrom dplyr sql_subquery
sql_subquery.DBIConnection <- function(con,
from,
name = unique_subquery_name(con),
name = unique_subquery_name(),
...,
lvl = 0) {
sql_query_wrap(con, from = from, name = name, ..., lvl = lvl)
Expand Down
2 changes: 1 addition & 1 deletion R/lazy-select-query.R
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ sql_build.lazy_select_query <- function(op, con, ..., sql_options = NULL) {
inform(op$message_summarise)
}

alias <- remote_name(op$x, null_if_local = FALSE) %||% unique_subquery_name(con)
alias <- remote_name(op$x, null_if_local = FALSE) %||% unique_subquery_name()
from <- sql_build(op$x, con, sql_options = sql_options)
select_sql_list <- get_select_sql(
select = op$select,
Expand Down
2 changes: 1 addition & 1 deletion R/query-join.R
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ flatten_query.multi_join_query <- function(qry, query_list, con) {
}

# TODO reuse query
name <- unique_subquery_name(con)
name <- as_table_name(unique_subquery_name(), con)
wrapped_query <- set_names(list(qry), name)

query_list$queries <- c(query_list_new$queries, wrapped_query)
Expand Down
2 changes: 1 addition & 1 deletion R/query-set-op.R
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ flatten_query.union_query <- function(qry, query_list, con) {
}

# TODO reuse query
name <- unique_subquery_name(con)
name <- as_table_name(unique_subquery_name(), con)
wrapped_query <- set_names(list(qry), name)

query_list$queries <- c(query_list_new$queries, wrapped_query)
Expand Down
2 changes: 1 addition & 1 deletion R/sql-build.R
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ querylist_reuse_query <- function(qry, query_list, con) {
if (!is.na(id)) {
query_list$name <- names(query_list$queries)[[id]]
} else {
name <- unique_subquery_name(con)
name <- as_table_name(unique_subquery_name(), con)
wrapped_query <- set_names(list(qry), name)
query_list$queries <- c(query_list$queries, wrapped_query)
query_list$name <- name
Expand Down
2 changes: 1 addition & 1 deletion R/table-name.R
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ as_table_name <- function(x,
table_name(unclass(x))
} else if (is.character(x)) {
check_string(x, allow_empty = FALSE, arg = error_arg, call = error_call)
make_table_name(x, con, collapse = FALSE)
make_table_name(unname(x), con, collapse = FALSE)
} else {
cli::cli_abort(
"{.arg {error_arg}} uses unknown specification for table name",
Expand Down
4 changes: 2 additions & 2 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ unique_table_name <- function() {
options(dbplyr_table_name = i)
sprintf("dbplyr_%03i", i)
}
unique_subquery_name <- function(con) {
unique_subquery_name <- function() {
# Needs to use option so can reset at the start of each query
i <- getOption("dbplyr_subquery_name", 0) + 1
options(dbplyr_subquery_name = i)
as_table_name(sprintf("q%02i", i), con)
sprintf("q%02i", i)
}
unique_column_name <- function() {
# Needs to use option so can reset at the start of each query
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/test-table-name.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ test_that("can coerce all user facing inputs", {
expect_equal(as_table_name(id, con), table_name("`foo`.`bar`.`baz`"))
})

test_that("strips names", {
con <- simulate_dbi()
expect_equal(as_table_name(c(x = "x"), con), table_name("`x`"))
})

test_that("as_table_name validates its inputs", {
con <- simulate_dbi()
expect_snapshot(error = TRUE, {
Expand Down
3 changes: 1 addition & 2 deletions tests/testthat/test-verb-compute.R
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,9 @@ test_that("compute keeps window and groups", {

test_that("compute can handle named name", {
con <- simulate_dbi()
name <- set_names(unique_subquery_name(con), unique_subquery_name(con))
expect_equal(
memdb_frame(x = 1:10) %>%
compute() %>%
compute(name = c(x = unique_table_name())) %>%
collect(),
tibble(x = 1:10)
)
Expand Down

0 comments on commit bff0130

Please sign in to comment.