From d530da043bbd793ebfcd4c475982a306e49257d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 5 Oct 2023 08:49:38 +0200 Subject: [PATCH 1/7] Replace build_copy_queries() with dm_sql() --- R/db-interface.R | 173 +++++++++++--------------- man/copy_dm_to.Rd | 41 +----- tests/testthat/_snaps/db-interface.md | 9 ++ tests/testthat/_snaps/learn.md | 74 +++++++++-- tests/testthat/test-db-interface.R | 9 +- 5 files changed, 149 insertions(+), 157 deletions(-) diff --git a/R/db-interface.R b/R/db-interface.R index b928ae3032..6da57f7396 100644 --- a/R/db-interface.R +++ b/R/db-interface.R @@ -5,14 +5,13 @@ #' and a [`dm`] object as its second argument. #' The latter is copied to the former. #' The default is to create temporary tables, set `temporary = FALSE` to create permanent tables. -#' Unless `set_key_constraints` is `FALSE`, primary key constraints are set on all databases, -#' and in addition foreign key constraints are set on MSSQL and Postgres databases. +#' Unless `set_key_constraints` is `FALSE`, primary key, foreign key, and unique constraints +#' are set on all databases. #' #' @inheritParams dm_examine_constraints #' #' @param dest An object of class `"src"` or `"DBIConnection"`. #' @param dm A `dm` object. -#' @inheritParams rlang::args_dots_empty #' @param set_key_constraints If `TRUE` will mirror `dm` primary and foreign key constraints on a database #' and create indexes for foreign key constraints. #' Set to `FALSE` if your data model currently does not satisfy primary or foreign key constraints. @@ -23,38 +22,9 @@ #' `table_names` is not `NULL`. #' #' Not all DBMS are supported. -#' @param table_names Desired names for the tables on `dest`; the names within the `dm` remain unchanged. -#' Can be `NULL`, a named character vector, or a vector of [DBI::Id] objects. -#' -#' If left `NULL` (default), the names will be determined automatically depending on the `temporary` argument: -#' -#' 1. `temporary = TRUE` (default): unique table names based on the names of the tables in the `dm` are created. -#' 1. `temporary = FALSE`: the table names in the `dm` are used as names for the tables on `dest`. -#' -#' If a function or one-sided formula, `table_names` is converted to a function -#' using [rlang::as_function()]. -#' This function is called with the unquoted table names of the `dm` object -#' as the only argument. -#' The output of this function is processed by [DBI::dbQuoteIdentifier()], -#' that result should be a vector of identifiers of the same length -#' as the original table names. -#' -#' Use a variant of -#' `table_names = ~ DBI::SQL(paste0("schema_name", ".", .x))` -#' to specify the same schema for all tables. -#' Use `table_names = identity` with `temporary = TRUE` -#' to avoid giving temporary tables unique names. -#' -#' If a named character vector, -#' the names of this vector need to correspond to the table names in the `dm`, -#' and its values are the desired names on `dest`. -#' The value is processed by [DBI::dbQuoteIdentifier()], -#' that result should be a vector of identifiers of the same length -#' as the original table names. -#' -#' Use qualified names corresponding to your database's syntax -#' to specify e.g. database and schema for your tables. -#' @param unique_table_names,copy_to Must be `NULL`. +#' @inheritParams dm_sql +#' @inheritParams rlang::args_dots_empty +#' @param unique_table_names,copy_to Deprecated. #' #' @family DB interaction functions #' @@ -100,14 +70,10 @@ copy_dm_to <- function( # 3. implement the key situation within our `dm` on the DB if (!is.null(unique_table_names)) { - deprecate_warn( + deprecate_stop( "0.1.4", "dm::copy_dm_to(unique_table_names = )", - details = "Use `table_names = identity` to use unchanged names for temporary tables." + details = "Use `table_names = set_names(names(dm))` to use unchanged names for temporary tables." ) - - if (is.null(table_names) && temporary && !unique_table_names) { - table_names <- identity - } } if (!is.null(copy_to)) { @@ -124,113 +90,114 @@ copy_dm_to <- function( check_suggested("dbplyr", "copy_dm_to") dest <- src_from_src_or_con(dest) - src_names <- src_tbls_impl(dm) - - if (is_db(dest)) { - dest_con <- con_from_src_or_con(dest) - - # in case `table_names` was chosen by the user, check if the input makes sense: - # 1. is there one name per dm-table? - # 2. are there any duplicated table names? - # 3. is it a named character or ident_q vector with the correct names? - if (is.null(table_names)) { - table_names_out <- repair_table_names_for_db(src_names, temporary, dest_con, schema) - # https://github.com/tidyverse/dbplyr/issues/487 - if (is_mssql(dest)) { - temporary <- FALSE - } - } else { - if (!is.null(schema)) abort_one_of_schema_table_names() - if (is_function(table_names) || is_bare_formula(table_names)) { - table_name_fun <- as_function(table_names) - table_names_out <- set_names(table_name_fun(src_names), src_names) - } else { - table_names_out <- table_names - } - check_naming(names(table_names_out), src_names) - - if (anyDuplicated(table_names_out)) { - problem <- table_names_out[duplicated(table_names_out)][[1]] - abort_copy_dm_to_table_names_duplicated(problem) - } - names(table_names_out) <- src_names - } - } else { - # FIXME: Other data sources than local and database possible - deprecate_warn( - "0.1.6", "dm::copy_dm_to(dest = 'must refer to a remote data source')", + if (!is_db(dest)) { + deprecate_stop( + "0.1.6", "dm::copy_dm_to(dest = 'must refer to a DBI connection')", "dm::collect.dm()" ) - table_names_out <- set_names(src_names) } - # FIXME: if same_src(), can use compute() but need to set NOT NULL and other - # constraints + src_names <- src_tbls_impl(dm) + dest_con <- con_from_src_or_con(dest) + + # in case `table_names` was chosen by the user, check if the input makes sense: + # 1. is there one name per dm-table? + # 2. are there any duplicated table names? + # 3. is it a named character or ident_q vector with the correct names? + if (is.null(table_names)) { + table_names_out <- repair_table_names_for_db(src_names, temporary, dest_con, schema) + # https://github.com/tidyverse/dbplyr/issues/487 + if (is_mssql(dest)) { + temporary <- FALSE + } + } else { + if (!is.null(schema)) abort_one_of_schema_table_names() + if (is_function(table_names) || is_bare_formula(table_names)) { + table_name_fun <- as_function(table_names) + table_names_out <- set_names(table_name_fun(src_names), src_names) + } else { + table_names_out <- table_names + } + check_naming(names(table_names_out), src_names) + + if (anyDuplicated(table_names_out)) { + problem <- table_names_out[duplicated(table_names_out)][[1]] + abort_copy_dm_to_table_names_duplicated(problem) + } - # Shortcut necessary to avoid copying into .GlobalEnv - if (!is_db(dest)) { - return(dm) + names(table_names_out) <- src_names } # Must be done here because table types may depend on string length, #2066 dm <- collect(dm, progress = progress) - queries <- build_copy_queries(dest_con, dm, set_key_constraints, temporary, table_names_out) + if (isTRUE(set_key_constraints)) { + dm_for_sql <- dm + } else { + def_no_keys <- dm_get_def(dm) + def_no_keys$uks[] <- list(new_uk()) + def_no_keys$fks[] <- list(new_fk()) + # Must keep primary keys + dm_for_sql <- dm_from_def(def_no_keys) + } + + sql <- dm_sql(dm_for_sql, dest_con, table_names_out, temporary) + + # FIXME: Extract function + # FIXME: Make descriptions part of the dm_sql() output + + pre <- unlist(sql$pre) + load <- unlist(sql$load) + post <- unlist(sql$post) - ticker_create <- new_ticker( + ticker_pre <- new_ticker( "creating tables", - n = length(queries$sql_table), + n = length(pre), progress = progress, top_level_fun = "copy_dm_to" ) # create tables - walk(queries$sql_table, ticker_create(~ { + walk(pre, ticker_pre(~ { DBI::dbExecute(dest_con, .x, immediate = TRUE) })) - ticker_populate <- new_ticker( + ticker_load <- new_ticker( "populating tables", - n = length(queries$name), + n = length(load), progress = progress, top_level_fun = "copy_dm_to" ) # populate tables - pwalk( - queries[c("name", "remote_name")], - ticker_populate(~ db_append_table( - con = dest_con, - remote_table = .y, - table = dm[[.x]], - progress = progress, - autoinc = dm_get_all_pks(dm, table = !!.x)$autoincrement - )) - ) + walk(load, ticker_load(~ { + DBI::dbExecute(dest_con, .x, immediate = TRUE) + })) - ticker_index <- new_ticker( + ticker_post <- new_ticker( "creating indexes", - n = sum(lengths(queries$sql_index)), + n = length(post), progress = progress, top_level_fun = "copy_dm_to" ) # create indexes - walk(unlist(queries$sql_index), ticker_index(~ { + walk(post, ticker_post(~ { DBI::dbExecute(dest_con, .x, immediate = TRUE) })) # remote dm is same as source dm with replaced data + # FIXME: Extract function def <- dm_get_def(dm) remote_tables <- map2( table_names_out, map(def$data, colnames), - ~ tbl(dest_con, ..1, vars = ..2) + ~ tbl(dest_con, .x, vars = .y) ) - def$data <- unname(remote_tables[names(dm)]) + def$data <- unname(remote_tables) remote_dm <- dm_from_def(def) invisible(debug_dm_validate(remote_dm)) diff --git a/man/copy_dm_to.Rd b/man/copy_dm_to.Rd index b39894b861..6b1ab02cea 100644 --- a/man/copy_dm_to.Rd +++ b/man/copy_dm_to.Rd @@ -28,38 +28,11 @@ copy_dm_to( and create indexes for foreign key constraints. Set to \code{FALSE} if your data model currently does not satisfy primary or foreign key constraints.} -\item{table_names}{Desired names for the tables on \code{dest}; the names within the \code{dm} remain unchanged. -Can be \code{NULL}, a named character vector, or a vector of \link[DBI:Id]{DBI::Id} objects. +\item{unique_table_names, copy_to}{Deprecated.} -If left \code{NULL} (default), the names will be determined automatically depending on the \code{temporary} argument: -\enumerate{ -\item \code{temporary = TRUE} (default): unique table names based on the names of the tables in the \code{dm} are created. -\item \code{temporary = FALSE}: the table names in the \code{dm} are used as names for the tables on \code{dest}. -} - -If a function or one-sided formula, \code{table_names} is converted to a function -using \code{\link[rlang:as_function]{rlang::as_function()}}. -This function is called with the unquoted table names of the \code{dm} object -as the only argument. -The output of this function is processed by \code{\link[DBI:dbQuoteIdentifier]{DBI::dbQuoteIdentifier()}}, -that result should be a vector of identifiers of the same length -as the original table names. - -Use a variant of -\code{table_names = ~ DBI::SQL(paste0("schema_name", ".", .x))} -to specify the same schema for all tables. -Use \code{table_names = identity} with \code{temporary = TRUE} -to avoid giving temporary tables unique names. - -If a named character vector, -the names of this vector need to correspond to the table names in the \code{dm}, -and its values are the desired names on \code{dest}. -The value is processed by \code{\link[DBI:dbQuoteIdentifier]{DBI::dbQuoteIdentifier()}}, -that result should be a vector of identifiers of the same length -as the original table names. - -Use qualified names corresponding to your database's syntax -to specify e.g. database and schema for your tables.} +\item{table_names}{A named character vector or vector of \link[DBI:Id]{DBI::Id} objects, +with one unique element for each table in \code{dm}. +The default, \code{NULL}, means to use the original table names.} \item{temporary}{If \code{TRUE}, only temporary tables will be created. These tables will vanish when disconnecting from the database.} @@ -73,8 +46,6 @@ Not all DBMS are supported.} \item{progress}{Whether to display a progress bar, if \code{NA} (the default) hide in non-interactive mode, show in interactive mode. Requires the 'progress' package.} - -\item{unique_table_names, copy_to}{Must be \code{NULL}.} } \value{ A \code{dm} object on the given \code{src} with the same table names @@ -85,8 +56,8 @@ as the input \code{dm}. and a \code{\link{dm}} object as its second argument. The latter is copied to the former. The default is to create temporary tables, set \code{temporary = FALSE} to create permanent tables. -Unless \code{set_key_constraints} is \code{FALSE}, primary key constraints are set on all databases, -and in addition foreign key constraints are set on MSSQL and Postgres databases. +Unless \code{set_key_constraints} is \code{FALSE}, primary key, foreign key, and unique constraints +are set on all databases. } \examples{ \dontshow{if (rlang::is_installed(c("RSQLite", "nycflights13", "dbplyr"))) (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} diff --git a/tests/testthat/_snaps/db-interface.md b/tests/testthat/_snaps/db-interface.md index 92b35875e0..fc2e5c5671 100644 --- a/tests/testthat/_snaps/db-interface.md +++ b/tests/testthat/_snaps/db-interface.md @@ -1,3 +1,12 @@ +# copy_dm_to() copies data frames from any source + + Code + copy_dm_to(default_local_src(), dm_for_filter()) + Condition + Error: + ! The `dest` argument of `copy_dm_to()` must refer to a DBI connection as of dm 0.1.6. + i Please use `collect.dm()` instead. + # copy_dm_to() rejects overwrite and types arguments Code diff --git a/tests/testthat/_snaps/learn.md b/tests/testthat/_snaps/learn.md index 2f17067ceb..189efc9da3 100644 --- a/tests/testthat/_snaps/learn.md +++ b/tests/testthat/_snaps/learn.md @@ -264,84 +264,132 @@ }, { "constraint_name": 2, + "table_name": "tf_1", + "column_name": "a", + "ordinal_position": 1 + }, + { + "constraint_name": 3, "table_name": "tf_2", "column_name": "c", "ordinal_position": 1 }, { - "constraint_name": 3, + "constraint_name": 4, + "table_name": "tf_2", + "column_name": "c", + "ordinal_position": 1 + }, + { + "constraint_name": 5, "table_name": "tf_2", "column_name": "d", "ordinal_position": 1 }, { - "constraint_name": 4, + "constraint_name": 6, "table_name": "tf_2", "column_name": "e", "ordinal_position": 1 }, { - "constraint_name": 4, + "constraint_name": 6, "table_name": "tf_2", "column_name": "e1", "ordinal_position": 2 }, { - "constraint_name": 5, + "constraint_name": 7, "table_name": "tf_3", "column_name": "f", "ordinal_position": 1 }, { - "constraint_name": 5, + "constraint_name": 7, "table_name": "tf_3", "column_name": "f1", "ordinal_position": 2 }, { - "constraint_name": 6, + "constraint_name": 8, + "table_name": "tf_3", + "column_name": "f", + "ordinal_position": 1 + }, + { + "constraint_name": 8, + "table_name": "tf_3", + "column_name": "f1", + "ordinal_position": 2 + }, + { + "constraint_name": 9, + "table_name": "tf_3", + "column_name": "g", + "ordinal_position": 1 + }, + { + "constraint_name": 10, "table_name": "tf_4", "column_name": "h", "ordinal_position": 1 }, { - "constraint_name": 7, + "constraint_name": 11, + "table_name": "tf_4", + "column_name": "h", + "ordinal_position": 1 + }, + { + "constraint_name": 12, "table_name": "tf_4", "column_name": "j", "ordinal_position": 1 }, { - "constraint_name": 7, + "constraint_name": 12, "table_name": "tf_4", "column_name": "j1", "ordinal_position": 2 }, { - "constraint_name": 8, + "constraint_name": 13, "table_name": "tf_5", "column_name": "k", "ordinal_position": 1 }, { - "constraint_name": 9, + "constraint_name": 14, + "table_name": "tf_5", + "column_name": "k", + "ordinal_position": 1 + }, + { + "constraint_name": 15, "table_name": "tf_5", "column_name": "l", "ordinal_position": 1 }, { - "constraint_name": 10, + "constraint_name": 16, "table_name": "tf_5", "column_name": "m", "ordinal_position": 1 }, { - "constraint_name": 11, + "constraint_name": 17, "table_name": "tf_6", "column_name": "n", "ordinal_position": 1 }, { - "constraint_name": 12, + "constraint_name": 18, + "table_name": "tf_6", + "column_name": "o", + "ordinal_position": 1 + }, + { + "constraint_name": 19, "table_name": "tf_6", "column_name": "o", "ordinal_position": 1 diff --git a/tests/testthat/test-db-interface.R b/tests/testthat/test-db-interface.R index 3f3608422f..357121c0f9 100644 --- a/tests/testthat/test-db-interface.R +++ b/tests/testthat/test-db-interface.R @@ -20,12 +20,9 @@ test_that("copy_dm_to() copies data frames to databases", { }) test_that("copy_dm_to() copies data frames from any source", { - expect_equivalent_dm( - expect_deprecated_obj( - copy_dm_to(default_local_src(), dm_for_filter()) - ), - dm_for_filter() - ) + expect_snapshot(error = TRUE, { + copy_dm_to(default_local_src(), dm_for_filter()) + }) }) # FIXME: Add test that set_key_constraints = FALSE doesn't set key constraints, From 01ce24b7f7ab7aa7374705f1c34de726a829e3da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Mon, 9 Oct 2023 17:04:10 +0200 Subject: [PATCH 2/7] Tweak docs --- R/db-interface.R | 53 ++++++++++++++++++++++++++++++++++++----------- man/copy_dm_to.Rd | 48 +++++++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/R/db-interface.R b/R/db-interface.R index 6da57f7396..af0496613e 100644 --- a/R/db-interface.R +++ b/R/db-interface.R @@ -6,15 +6,19 @@ #' The latter is copied to the former. #' The default is to create temporary tables, set `temporary = FALSE` to create permanent tables. #' Unless `set_key_constraints` is `FALSE`, primary key, foreign key, and unique constraints -#' are set on all databases. +#' are set, and indexes for foreign keys are created, on all databases. #' #' @inheritParams dm_examine_constraints #' #' @param dest An object of class `"src"` or `"DBIConnection"`. #' @param dm A `dm` object. -#' @param set_key_constraints If `TRUE` will mirror `dm` primary and foreign key constraints on a database -#' and create indexes for foreign key constraints. -#' Set to `FALSE` if your data model currently does not satisfy primary or foreign key constraints. +#' @inheritParams rlang::args_dots_empty +#' @param set_key_constraints If `TRUE` will mirror +#' the primary, foreign, and unique key constraints +#' and create indexes for foreign key constraints +#' for the primary and foreign keys in the `dm` object. +#' Set to `FALSE` if your data model currently does not satisfy +#' primary or foreign key constraints. #' @param temporary If `TRUE`, only temporary tables will be created. #' These tables will vanish when disconnecting from the database. #' @param schema Name of schema to copy the `dm` to. @@ -22,9 +26,38 @@ #' `table_names` is not `NULL`. #' #' Not all DBMS are supported. -#' @inheritParams dm_sql -#' @inheritParams rlang::args_dots_empty -#' @param unique_table_names,copy_to Deprecated. +#' @param table_names Desired names for the tables on `dest`; the names within the `dm` remain unchanged. +#' Can be `NULL`, a named character vector, or a vector of [DBI::Id] objects. +#' +#' If left `NULL` (default), the names will be determined automatically depending on the `temporary` argument: +#' +#' 1. `temporary = TRUE` (default): unique table names based on the names of the tables in the `dm` are created. +#' 1. `temporary = FALSE`: the table names in the `dm` are used as names for the tables on `dest`. +#' +#' If a function or one-sided formula, `table_names` is converted to a function +#' using [rlang::as_function()]. +#' This function is called with the unquoted table names of the `dm` object +#' as the only argument. +#' The output of this function is processed by [DBI::dbQuoteIdentifier()], +#' that result should be a vector of identifiers of the same length +#' as the original table names. +#' +#' Use a variant of +#' `table_names = ~ DBI::SQL(paste0("schema_name", ".", .x))` +#' to specify the same schema for all tables. +#' Use `table_names = identity` with `temporary = TRUE` +#' to avoid giving temporary tables unique names. +#' +#' If a named character vector, +#' the names of this vector need to correspond to the table names in the `dm`, +#' and its values are the desired names on `dest`. +#' The value is processed by [DBI::dbQuoteIdentifier()], +#' that result should be a vector of identifiers of the same length +#' as the original table names. +#' +#' Use qualified names corresponding to your database's syntax +#' to specify e.g. database and schema for your tables. +#' @param unique_table_names,copy_to Must be `NULL`. #' #' @family DB interaction functions #' @@ -63,11 +96,7 @@ copy_dm_to <- function( progress = NA, unique_table_names = NULL, copy_to = NULL) { - # for the time being, we will be focusing on MSSQL - # we want to - # 1. change `dm_get_src_impl(dm)` to `dest` - # 2. copy the tables to `dest` - # 3. implement the key situation within our `dm` on the DB + # if (!is.null(unique_table_names)) { deprecate_stop( diff --git a/man/copy_dm_to.Rd b/man/copy_dm_to.Rd index 6b1ab02cea..5f57acdab4 100644 --- a/man/copy_dm_to.Rd +++ b/man/copy_dm_to.Rd @@ -24,15 +24,45 @@ copy_dm_to( \item{...}{These dots are for future extensions and must be empty.} -\item{set_key_constraints}{If \code{TRUE} will mirror \code{dm} primary and foreign key constraints on a database -and create indexes for foreign key constraints. -Set to \code{FALSE} if your data model currently does not satisfy primary or foreign key constraints.} +\item{set_key_constraints}{If \code{TRUE} will mirror +the primary, foreign, and unique key constraints +and create indexes for foreign key constraints +for the primary and foreign keys in the \code{dm} object. +Set to \code{FALSE} if your data model currently does not satisfy +primary or foreign key constraints.} -\item{unique_table_names, copy_to}{Deprecated.} +\item{table_names}{Desired names for the tables on \code{dest}; the names within the \code{dm} remain unchanged. +Can be \code{NULL}, a named character vector, or a vector of \link[DBI:Id]{DBI::Id} objects. -\item{table_names}{A named character vector or vector of \link[DBI:Id]{DBI::Id} objects, -with one unique element for each table in \code{dm}. -The default, \code{NULL}, means to use the original table names.} +If left \code{NULL} (default), the names will be determined automatically depending on the \code{temporary} argument: +\enumerate{ +\item \code{temporary = TRUE} (default): unique table names based on the names of the tables in the \code{dm} are created. +\item \code{temporary = FALSE}: the table names in the \code{dm} are used as names for the tables on \code{dest}. +} + +If a function or one-sided formula, \code{table_names} is converted to a function +using \code{\link[rlang:as_function]{rlang::as_function()}}. +This function is called with the unquoted table names of the \code{dm} object +as the only argument. +The output of this function is processed by \code{\link[DBI:dbQuoteIdentifier]{DBI::dbQuoteIdentifier()}}, +that result should be a vector of identifiers of the same length +as the original table names. + +Use a variant of +\code{table_names = ~ DBI::SQL(paste0("schema_name", ".", .x))} +to specify the same schema for all tables. +Use \code{table_names = identity} with \code{temporary = TRUE} +to avoid giving temporary tables unique names. + +If a named character vector, +the names of this vector need to correspond to the table names in the \code{dm}, +and its values are the desired names on \code{dest}. +The value is processed by \code{\link[DBI:dbQuoteIdentifier]{DBI::dbQuoteIdentifier()}}, +that result should be a vector of identifiers of the same length +as the original table names. + +Use qualified names corresponding to your database's syntax +to specify e.g. database and schema for your tables.} \item{temporary}{If \code{TRUE}, only temporary tables will be created. These tables will vanish when disconnecting from the database.} @@ -46,6 +76,8 @@ Not all DBMS are supported.} \item{progress}{Whether to display a progress bar, if \code{NA} (the default) hide in non-interactive mode, show in interactive mode. Requires the 'progress' package.} + +\item{unique_table_names, copy_to}{Must be \code{NULL}.} } \value{ A \code{dm} object on the given \code{src} with the same table names @@ -57,7 +89,7 @@ and a \code{\link{dm}} object as its second argument. The latter is copied to the former. The default is to create temporary tables, set \code{temporary = FALSE} to create permanent tables. Unless \code{set_key_constraints} is \code{FALSE}, primary key, foreign key, and unique constraints -are set on all databases. +are set, and indexes for foreign keys are created, on all databases. } \examples{ \dontshow{if (rlang::is_installed(c("RSQLite", "nycflights13", "dbplyr"))) (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} From cc218db30319b5d67037d0431a7aebda162e722b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Mon, 9 Oct 2023 17:04:17 +0200 Subject: [PATCH 3/7] Also reset PKs --- R/db-interface.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/db-interface.R b/R/db-interface.R index af0496613e..414c2180f4 100644 --- a/R/db-interface.R +++ b/R/db-interface.R @@ -165,6 +165,8 @@ copy_dm_to <- function( dm_for_sql <- dm } else { def_no_keys <- dm_get_def(dm) + # FIXME: Add test + def_no_keys$pks[] <- list(new_pk()) def_no_keys$uks[] <- list(new_uk()) def_no_keys$fks[] <- list(new_fk()) # Must keep primary keys From a22c0a45af3a88a672cdb6534531217b7ae58fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 21 Dec 2023 23:48:04 +0100 Subject: [PATCH 4/7] Update snapshot --- tests/testthat/_snaps/filter-dm.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/testthat/_snaps/filter-dm.md b/tests/testthat/_snaps/filter-dm.md index d0fdc7c165..94ae32c9c2 100644 --- a/tests/testthat/_snaps/filter-dm.md +++ b/tests/testthat/_snaps/filter-dm.md @@ -223,6 +223,9 @@ Code dm_for_filter_rev() %>% dm_filter(tf_1 = a < 8 & a > 3) %>% dm_get_tables() %>% map(harmonize_tbl) + Condition + Warning: + Autoincrementing columns not yet supported for DuckDB, these won't be set in the remote database but are preserved in the `dm` Output $tf_6 # A tibble: 2 x 3 From 4836d17ac0c39f538ea0f0c5fbc09524949f0899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 21 Dec 2023 23:55:58 +0100 Subject: [PATCH 5/7] Try without parallel testing --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c2bd6fd55f..700f583176 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -118,7 +118,7 @@ VignetteBuilder: Config/autostyle/scope: line_breaks Config/autostyle/strict: true Config/testthat/edition: 3 -Config/testthat/parallel: true +Config/testthat/parallel: false Config/testthat/start-first: zzx-deprecated, flatten, dplyr, filter-dm, draw-dm, bind, rows-dm, learn Encoding: UTF-8 From 4accb53fe801e8369503c0a3f51dc474cb0c2673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Fri, 22 Dec 2023 00:02:43 +0100 Subject: [PATCH 6/7] Try without skip --- tests/testthat/test-filter-dm.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testthat/test-filter-dm.R b/tests/testthat/test-filter-dm.R index ded1d4092a..dafda764c8 100644 --- a/tests/testthat/test-filter-dm.R +++ b/tests/testthat/test-filter-dm.R @@ -44,8 +44,6 @@ test_that("dm_filter() legacy API", { test_that("dm_filter() deprecations", { local_options(lifecycle_verbosity = "warning") - skip_if_src_not("db") - expect_snapshot({ dm_filter(dm_for_filter(), tf_1, a > 4) dm_filter(dm = dm_for_filter(), tf_1, a > 4) From 112f6d19174cf133a619bfbe1301a6b8debe756d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 22 Dec 2023 15:15:06 +0100 Subject: [PATCH 7/7] Snapshot updates for rcc-smoke (null) (#2165) Co-authored-by: krlmlr --- tests/testthat/_snaps/filter-dm.md | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/testthat/_snaps/filter-dm.md b/tests/testthat/_snaps/filter-dm.md index 94ae32c9c2..177b237760 100644 --- a/tests/testthat/_snaps/filter-dm.md +++ b/tests/testthat/_snaps/filter-dm.md @@ -5,7 +5,7 @@ Condition Warning: The `table` argument of `dm_filter()` is deprecated as of dm 1.0.0. - `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. + i `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. Output -- Metadata -------------------------------------------------------------------- Tables: `tf_1`, `tf_2`, `tf_3`, `tf_4`, `tf_5`, `tf_6` @@ -19,10 +19,10 @@ Condition Warning: The `dm` argument of `dm_filter()` is deprecated as of dm 1.0.0. - Please use the `.dm` argument instead. + i Please use the `.dm` argument instead. Warning: The `table` argument of `dm_filter()` is deprecated as of dm 1.0.0. - `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. + i `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. Output -- Metadata -------------------------------------------------------------------- Tables: `tf_1`, `tf_2`, `tf_3`, `tf_4`, `tf_5`, `tf_6` @@ -36,7 +36,7 @@ Condition Warning: The `table` argument of `dm_filter()` is deprecated as of dm 1.0.0. - `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. + i `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. Output -- Metadata -------------------------------------------------------------------- Tables: `tf_1`, `tf_2`, `tf_3`, `tf_4`, `tf_5`, `tf_6` @@ -48,7 +48,7 @@ Condition Warning: `dm_apply_filters()` was deprecated in dm 1.0.0. - Calling `dm_apply_filters()` after `dm_filter()` is no longer necessary. + i Calling `dm_apply_filters()` after `dm_filter()` is no longer necessary. Output -- Metadata -------------------------------------------------------------------- Tables: `tf_1`, `tf_2`, `tf_3`, `tf_4`, `tf_5`, `tf_6` @@ -60,7 +60,7 @@ Condition Warning: The `table` argument of `dm_filter()` is deprecated as of dm 1.0.0. - `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. + i `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. Output # A tibble: 3 x 4 c d e e1 @@ -73,7 +73,7 @@ Condition Warning: `dm_apply_filters_to_tbl()` was deprecated in dm 1.0.0. - Access tables directly after `dm_filter()`. + i Access tables directly after `dm_filter()`. Output # A tibble: 3 x 4 c d e e1 @@ -86,7 +86,7 @@ Condition Warning: The `table` argument of `dm_filter()` is deprecated as of dm 1.0.0. - `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. + i `dm_filter()` now takes named filter expressions, the names correspond to the tables to be filtered. You no longer need to call `dm_apply_filters()` to materialize the filters. Output # A tibble: 1 x 3 table filter zoomed @@ -97,10 +97,10 @@ Condition Warning: `dm_get_filters()` was deprecated in dm 1.0.0. - Filter conditions are no longer stored with the dm object. + i Filter conditions are no longer stored with the dm object. Output # A tibble: 0 x 3 - # ... with 3 variables: table , filter , zoomed + # i 3 variables: table , filter , zoomed # data structure @@ -223,9 +223,6 @@ Code dm_for_filter_rev() %>% dm_filter(tf_1 = a < 8 & a > 3) %>% dm_get_tables() %>% map(harmonize_tbl) - Condition - Warning: - Autoincrementing columns not yet supported for DuckDB, these won't be set in the remote database but are preserved in the `dm` Output $tf_6 # A tibble: 2 x 3