From bff01305e016402c2acda9088ed1d303907d2cb6 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 1 Dec 2023 13:40:56 -0600 Subject: [PATCH] unique_subquery_name() no longer needs con --- R/backend-teradata.R | 2 +- R/db-sql.R | 6 +++--- R/lazy-select-query.R | 2 +- R/query-join.R | 2 +- R/query-set-op.R | 2 +- R/sql-build.R | 2 +- R/table-name.R | 2 +- R/utils.R | 4 ++-- tests/testthat/test-table-name.R | 5 +++++ tests/testthat/test-verb-compute.R | 3 +-- 10 files changed, 17 insertions(+), 13 deletions(-) diff --git a/R/backend-teradata.R b/R/backend-teradata.R index 425a2348b..fe2007ee4 100644 --- a/R/backend-teradata.R +++ b/R/backend-teradata.R @@ -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, diff --git a/R/db-sql.R b/R/db-sql.R index 5c201ab15..4eb03df13 100644 --- a/R/db-sql.R +++ b/R/db-sql.R @@ -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) @@ -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) diff --git a/R/lazy-select-query.R b/R/lazy-select-query.R index d07f36da2..84e81c9f0 100644 --- a/R/lazy-select-query.R +++ b/R/lazy-select-query.R @@ -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, diff --git a/R/query-join.R b/R/query-join.R index 9b44223f5..a51df1341 100644 --- a/R/query-join.R +++ b/R/query-join.R @@ -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) diff --git a/R/query-set-op.R b/R/query-set-op.R index 1f2566717..5571a03ee 100644 --- a/R/query-set-op.R +++ b/R/query-set-op.R @@ -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) diff --git a/R/sql-build.R b/R/sql-build.R index ccc662475..cd4d4b0f7 100644 --- a/R/sql-build.R +++ b/R/sql-build.R @@ -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 diff --git a/R/table-name.R b/R/table-name.R index 77dca7434..6c2b0f268 100644 --- a/R/table-name.R +++ b/R/table-name.R @@ -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", diff --git a/R/utils.R b/R/utils.R index cea2a1b9a..b82e3cbf5 100644 --- a/R/utils.R +++ b/R/utils.R @@ -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 diff --git a/tests/testthat/test-table-name.R b/tests/testthat/test-table-name.R index 36b5e16cc..9cbd513c7 100644 --- a/tests/testthat/test-table-name.R +++ b/tests/testthat/test-table-name.R @@ -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, { diff --git a/tests/testthat/test-verb-compute.R b/tests/testthat/test-verb-compute.R index 363cd5766..4dc1e2282 100644 --- a/tests/testthat/test-verb-compute.R +++ b/tests/testthat/test-verb-compute.R @@ -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) )