From 86aeea53915f89d4e04326a0061422d6bb614331 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 18 Dec 2023 17:17:32 -0600 Subject: [PATCH] Refine table_name_table() interface --- R/backend-postgres-old.R | 3 +-- R/db-sql.R | 4 ++-- R/lazy-join-query.R | 3 ++- R/query-join.R | 21 ++++++------------- R/remote.R | 2 +- R/table-name.R | 35 +++++++++++++++++--------------- R/utils.R | 2 +- tests/testthat/test-verb-joins.R | 8 ++++---- 8 files changed, 36 insertions(+), 42 deletions(-) diff --git a/R/backend-postgres-old.R b/R/backend-postgres-old.R index e16e3635b..885b92517 100644 --- a/R/backend-postgres-old.R +++ b/R/backend-postgres-old.R @@ -15,7 +15,6 @@ db_write_table.PostgreSQLConnection <- function(con, values, temporary = TRUE, ...) { - table <- as_table_name(table, con) if (!isFALSE(temporary)) { cli_abort(c( "RPostgreSQL backend does not support creation of temporary tables", @@ -27,7 +26,7 @@ db_write_table.PostgreSQLConnection <- function(con, # the bare table name dbWriteTable( con, - name = db_table_name_extract(con, table), + name = table_name_table(table, con), value = values, field.types = types, ..., diff --git a/R/db-sql.R b/R/db-sql.R index 4eb03df13..60b8c9455 100644 --- a/R/db-sql.R +++ b/R/db-sql.R @@ -176,7 +176,7 @@ sql_table_index.DBIConnection <- function(con, table <- as_table_name(table, con) if (is.null(name)) { - table_name <- db_table_name_extract(con, table) + table_name <- table_name_table(table, con) name <- name %||% paste0(c(table_name, columns), collapse = "_") } glue_sql2( @@ -259,7 +259,7 @@ sql_query_wrap.DBIConnection <- function(con, from, name = NULL, ..., lvl = 0) { 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) + table <- table_name_table(name, con) names(from) <- as_table_name(table, con) } from diff --git a/R/lazy-join-query.R b/R/lazy-join-query.R index 71f356130..ffce3a6f7 100644 --- a/R/lazy-join-query.R +++ b/R/lazy-join-query.R @@ -153,6 +153,7 @@ op_vars.lazy_semi_join_query <- function(op) { #' @export sql_build.lazy_multi_join_query <- function(op, con, ..., sql_options = NULL) { + table_names_out <- generate_join_table_names(op$table_names, con) tables <- set_names(c(list(op$x), op$joins$table), table_names_out) @@ -188,7 +189,7 @@ sql_build.lazy_multi_join_query <- function(op, con, ..., sql_options = NULL) { } generate_join_table_names <- function(table_names, con) { - names <- db_table_name_extract(con, table_names$name) + names <- table_name_table(table_names$name, con) table_name_length_max <- max(nchar(names)) if (length(table_names$name) != 2) { diff --git a/R/query-join.R b/R/query-join.R index a51df1341..0ef04e280 100644 --- a/R/query-join.R +++ b/R/query-join.R @@ -331,29 +331,20 @@ sql_join_tbls <- function(con, by, na_matches) { } sql_table_prefix <- function(con, var, table = NULL) { - if (!is_bare_character(var)) { cli_abort("{.arg var} must be a bare character.", .internal = TRUE) } var <- sql_escape_ident(con, var) - - if (!is.null(table)) { - table <- as_table_name(table, con) - table <- db_table_name_extract(con, table) - table <- as_table_name(table, con) - - sql(paste0(table, ".", var)) - } else { - var - } + sql_table_name_prefix(con, table, var) } sql_star <- function(con, table = NULL) { - # TODO: sql_table_prefix(con, "*", table) ? - var <- sql("*") + sql_table_name_prefix(con, table, sql("*")) +} + +sql_table_name_prefix <- function(con, table, var) { if (!is.null(table)) { - table <- as_table_name(table, con) - table <- db_table_name_extract(con, table) + table <- table_name_table(table, con) table <- as_table_name(table, con) sql(paste0(table, ".", var)) diff --git a/R/remote.R b/R/remote.R index ef176c9f9..b303ccc04 100644 --- a/R/remote.R +++ b/R/remote.R @@ -40,7 +40,7 @@ remote_name <- function(x, null_if_local = TRUE) { if (is.null(con)) { table } else { - db_table_name_extract(con, table) + table_name_table(table, con) } } } diff --git a/R/table-name.R b/R/table-name.R index 6c2b0f268..372c9ebe1 100644 --- a/R/table-name.R +++ b/R/table-name.R @@ -55,9 +55,20 @@ as_table_names <- function(x, con) { make_table_name(x, con, collapse = FALSE) } -# TODO: make this generic -db_parse_table_name <- function(con, x) { - quote_char <- substr( sql_escape_ident(con, ""), 1, 1) +# Extract just the table name from a full identifier +table_name_table <- function(x, con) { + x <- as_table_name(x, con) + + vapply(x, FUN.VALUE = character(1), function(x) { + if (x == "") return("") + + out <- table_name_components(x, con) + out[[length(out)]] + }) +} +table_name_components <- function(x, con) { + quote_char <- substr(sql_escape_ident(con, ""), 1, 1) + scan( text = x, what = character(), @@ -67,21 +78,13 @@ db_parse_table_name <- function(con, x) { sep = "." ) } -db_table_name_extract <- function(con, x) { - vapply(x, FUN.VALUE = character(1), function(x) { - if (x == "") return("") - - out <- db_parse_table_name(con, x) - out[[length(out)]] - }) -} #' @export escape.dbplyr_table_name <- function(x, parens = FALSE, collapse = ", ", con = NULL) { - alias <- names2(x) # assume alias is already escaped - x <- unname(x) - - table_name <- as_table_name(db_table_name_extract(con, x), con) + # names are always already escaped + alias <- names2(x) + table_name <- as_table_name(table_name_table(x, con), con) + has_alias <- alias == "" | alias == table_name if (db_supports_table_alias_with_as(con)) { as_sql <- style_kw(" AS ") @@ -89,7 +92,7 @@ escape.dbplyr_table_name <- function(x, parens = FALSE, collapse = ", ", con = N as_sql <- " " } - out <- ifelse(alias == "" | alias == table_name, x, paste0(x, as_sql, alias)) + out <- ifelse(has_alias, unname(x), paste0(x, as_sql, alias)) sql_vector(out, parens, collapse, con = con) } diff --git a/R/utils.R b/R/utils.R index b82e3cbf5..945039294 100644 --- a/R/utils.R +++ b/R/utils.R @@ -89,7 +89,7 @@ add_temporary_prefix <- function(con, table, temporary = TRUE) { return(table) } - pieces <- db_parse_table_name(con, table) + pieces <- table_name_components(table, con) table_name <- pieces[length(pieces)] if (substr(table_name, 1, 1) != "#") { diff --git a/tests/testthat/test-verb-joins.R b/tests/testthat/test-verb-joins.R index bf303ebcf..e8e833a88 100644 --- a/tests/testthat/test-verb-joins.R +++ b/tests/testthat/test-verb-joins.R @@ -102,11 +102,11 @@ test_that("alias truncates long table names at database limit", { # Source: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS con <- src_test("postgres") - nm1 <- paste0("a", paste0(0:61 %% 10, collapse = "")) - mf1 <- local_db_table(con, tibble(x = 1:3, y = "a"), nm1) + nm1 <- table_name(paste0("a", paste0(0:61 %% 10, collapse = ""))) + mf1 <- local_db_table(con, tibble(x = 1:3, y = "a"), unclass(nm1)) - nm2 <- paste0("b", paste0(0:61 %% 10, collapse = "")) - mf2 <- local_db_table(con, tibble(x = 2:3, y = "b"), nm2) + nm2 <- table_name(paste0("b", paste0(0:61 %% 10, collapse = ""))) + mf2 <- local_db_table(con, tibble(x = 2:3, y = "b"), unclass(nm2)) # 2 tables # aliased names are as expected