From 5a04aeee7e542c44258b852d45109525e1059ba5 Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 12:09:39 -0500 Subject: [PATCH 1/3] add live warehouse test helpers --- DESCRIPTION | 1 + tests/testthat/README.md | 31 ++++++++++++++ tests/testthat/helper-live-warehouses.R | 55 +++++++++++++++++++++++++ tests/testthat/test-live-warehouses.R | 44 ++++++++++++++++++++ 4 files changed, 131 insertions(+) create mode 100644 tests/testthat/README.md create mode 100644 tests/testthat/helper-live-warehouses.R create mode 100644 tests/testthat/test-live-warehouses.R diff --git a/DESCRIPTION b/DESCRIPTION index 4691d14..ba580cb 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -43,6 +43,7 @@ Suggests: dbplyr, dplyr, htmltools, + odbc, otel (>= 0.2.0), otelsdk (>= 0.2.0), pins, diff --git a/tests/testthat/README.md b/tests/testthat/README.md new file mode 100644 index 0000000..c510119 --- /dev/null +++ b/tests/testthat/README.md @@ -0,0 +1,31 @@ +# Live warehouse tests + +The Snowflake and Databricks smoke tests are opt in. The ordinary test suite +skips them before connecting, so contributors do not need warehouse credentials +or ODBC drivers. + +To run the Snowflake test, configure `odbc::snowflake()` as usual and set: + +```sh +export COMMONS_LIVE_SNOWFLAKE=true +export COMMONS_SNOWFLAKE_DATABASE=... +export COMMONS_SNOWFLAKE_SCHEMA=... +export COMMONS_SNOWFLAKE_TABLE=... +``` + +To run the Databricks test, configure an ODBC DSN named `Databricks` and set: + +```sh +export COMMONS_LIVE_DATABRICKS=true +export COMMONS_DATABRICKS_CATALOG=... +export COMMONS_DATABRICKS_SCHEMA=... +export COMMONS_DATABRICKS_TABLE=... +``` + +Set `COMMONS_DATABRICKS_DSN` to use a differently named DSN. Identifiers are +passed as separate `catalog`, `schema`, and `table` components of `DBI::Id()`; +do not combine them into a dotted string. + +Each test queries the current identity and namespace, then reads at most one row +from the configured table. If a live-test switch or identifier is absent, that +backend's test skips. Once enabled, connection and query failures fail the test. diff --git a/tests/testthat/helper-live-warehouses.R b/tests/testthat/helper-live-warehouses.R new file mode 100644 index 0000000..47b0ab5 --- /dev/null +++ b/tests/testthat/helper-live-warehouses.R @@ -0,0 +1,55 @@ +local_warehouse_connection <- function(backend, env = parent.frame()) { + backend <- match.arg(backend, c("snowflake", "databricks")) + skip_unless_live_warehouse(backend) + skip_if_not_installed("odbc") + + con <- switch( + backend, + snowflake = DBI::dbConnect(odbc::snowflake()), + databricks = DBI::dbConnect( + odbc::odbc(), + Sys.getenv("COMMONS_DATABRICKS_DSN", unset = "Databricks") + ) + ) + withr::defer(DBI::dbDisconnect(con), envir = env) + con +} + +warehouse_test_objects <- function(backend) { + backend <- match.arg(backend, c("snowflake", "databricks")) + skip_unless_live_warehouse(backend) + + prefix <- toupper(backend) + top_level <- if (identical(backend, "snowflake")) "DATABASE" else "CATALOG" + names <- paste0( + "COMMONS_", prefix, "_", c(top_level, "SCHEMA", "TABLE") + ) + values <- Sys.getenv(names, unset = NA_character_) + missing <- names[is.na(values) | !nzchar(values)] + if (length(missing)) { + skip(paste("Missing live warehouse configuration:", paste(missing, collapse = ", "))) + } + + list(table = DBI::Id( + catalog = unname(values[[1]]), + schema = unname(values[[2]]), + table = unname(values[[3]]) + )) +} + +skip_unless_live_warehouse <- function(backend) { + variable <- paste0("COMMONS_LIVE_", toupper(backend)) + skip_if_not( + identical(tolower(Sys.getenv(variable)), "true"), + paste0("Set ", variable, "=true to run live warehouse tests") + ) +} + +warehouse_read_one <- function(con, id) { + sql <- paste( + "SELECT * FROM", + DBI::dbQuoteIdentifier(con, id), + "LIMIT 1" + ) + DBI::dbGetQuery(con, sql) +} diff --git a/tests/testthat/test-live-warehouses.R b/tests/testthat/test-live-warehouses.R new file mode 100644 index 0000000..dae3ecc --- /dev/null +++ b/tests/testthat/test-live-warehouses.R @@ -0,0 +1,44 @@ +test_that("live Snowflake connection reads a configured table", { + objects <- warehouse_test_objects("snowflake") + con <- local_warehouse_connection("snowflake") + + session <- DBI::dbGetQuery( + con, + paste( + "SELECT CURRENT_USER() AS principal,", + "CURRENT_ROLE() AS role,", + "CURRENT_DATABASE() AS catalog,", + "CURRENT_SCHEMA() AS schema" + ) + ) + rows <- warehouse_read_one(con, objects$table) + names(session) <- tolower(names(session)) + + expect_equal(nrow(session), 1) + expect_named(session, c("principal", "role", "catalog", "schema")) + expect_true(nzchar(session$principal[[1]])) + expect_s3_class(rows, "data.frame") + expect_true(nrow(rows) <= 1) +}) + +test_that("live Databricks connection reads a configured table", { + objects <- warehouse_test_objects("databricks") + con <- local_warehouse_connection("databricks") + + session <- DBI::dbGetQuery( + con, + paste( + "SELECT CURRENT_USER() AS principal,", + "CURRENT_CATALOG() AS catalog,", + "CURRENT_SCHEMA() AS schema" + ) + ) + rows <- warehouse_read_one(con, objects$table) + names(session) <- tolower(names(session)) + + expect_equal(nrow(session), 1) + expect_named(session, c("principal", "catalog", "schema")) + expect_true(nzchar(session$principal[[1]])) + expect_s3_class(rows, "data.frame") + expect_true(nrow(rows) <= 1) +}) From 169573f03a601d1b66189f60717ee73f11303f91 Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 12:21:44 -0500 Subject: [PATCH 2/3] add database source options --- NAMESPACE | 1 + R/data-source-options.R | 151 ++++++++++++++++++++++ R/data-source.R | 70 ++++++++-- man/data_source.Rd | 6 +- man/data_source_options.Rd | 25 ++++ tests/testthat/README.md | 7 +- tests/testthat/helper-live-warehouses.R | 9 -- tests/testthat/test-data-source-options.R | 116 +++++++++++++++++ tests/testthat/test-live-warehouses.R | 16 ++- 9 files changed, 372 insertions(+), 29 deletions(-) create mode 100644 R/data-source-options.R create mode 100644 man/data_source_options.Rd create mode 100644 tests/testthat/test-data-source-options.R diff --git a/NAMESPACE b/NAMESPACE index 6b0b410..a0452c5 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -5,6 +5,7 @@ export(commons_server) export(commons_ui) export(context_layer) export(data_source) +export(data_source_options) export(list_tables) export(measure) export(semantic_layer) diff --git a/R/data-source-options.R b/R/data-source-options.R new file mode 100644 index 0000000..51edb6d --- /dev/null +++ b/R/data-source-options.R @@ -0,0 +1,151 @@ +#' Configure a database data source +#' +#' `data_source_options()` controls which objects a DBI-backed [data_source()] +#' exposes. Character values select literal table names. Use [DBI::Id()] to +#' preserve catalog and schema components or to select a name containing dots. +#' +#' @param include Exact tables to expose: a character vector, a [DBI::Id()], or +#' a list of `DBI::Id()` objects. An ID without a `table` component represents +#' a namespace; namespace expansion requires backend catalog support and is +#' not yet available. +#' @param sample_rows A non-negative integer recording how many rows to sample +#' when describing a table. Table descriptions do not yet apply this value. +#' +#' @return A `commons_data_source_options` object for [data_source()]. +#' @export +data_source_options <- function(include = NULL, sample_rows = 0L) { + structure( + list( + include = normalize_connection_includes(include), + sample_rows = check_sample_rows(sample_rows) + ), + class = "commons_data_source_options" + ) +} + +check_data_source_options <- function( + options, + tables, + kind, + call = rlang::caller_env() +) { + if (is.null(options)) { + return(NULL) + } + if (!identical(kind, "connection")) { + cli::cli_abort( + "{.arg options} can only be used with a DBI connection.", + call = call + ) + } + if (!is.null(tables)) { + cli::cli_abort( + "Supply only one of {.arg tables} and {.arg options}.", + call = call + ) + } + if (!inherits(options, "commons_data_source_options")) { + cli::cli_abort( + "{.arg options} must come from {.fn data_source_options}.", + call = call + ) + } + options +} + +normalize_connection_includes <- function( + include, + call = rlang::caller_env() +) { + if (is.null(include)) { + return(NULL) + } + + entries <- if (inherits(include, "Id")) { + list(include) + } else if (is.character(include)) { + if (anyNA(include) || any(!nzchar(include))) { + cli::cli_abort( + "Character entries in {.arg include} must be non-empty table names.", + call = call + ) + } + lapply(include, function(table) DBI::Id(table = table)) + } else if (is.list(include)) { + include + } else { + cli::cli_abort( + paste0( + "{.arg include} must be a character vector, a {.cls DBI::Id}, ", + "or a list of {.cls DBI::Id} objects." + ), + call = call + ) + } + + if (length(entries) == 0) { + cli::cli_abort( + "{.arg include} must select at least one object.", + call = call + ) + } + if (!all(vapply(entries, inherits, logical(1), "Id"))) { + cli::cli_abort( + "Every entry in {.arg include} must be a {.cls DBI::Id}.", + call = call + ) + } + + for (id in entries) { + components <- id@name + if (any(is.na(components) | components == "")) { + cli::cli_abort( + "{.cls DBI::Id} entries in {.arg include} must have non-empty components.", + call = call + ) + } + } + + entries +} + +check_sample_rows <- function(sample_rows, call = rlang::caller_env()) { + valid <- is.numeric(sample_rows) && + length(sample_rows) == 1 && + !is.na(sample_rows) && + is.finite(sample_rows) && + sample_rows >= 0 && + sample_rows <= .Machine$integer.max && + sample_rows == as.integer(sample_rows) + if (!valid) { + cli::cli_abort( + "{.arg sample_rows} must be one non-negative integer.", + call = call + ) + } + as.integer(sample_rows) +} + +check_exact_connection_includes <- function( + include, + call = rlang::caller_env() +) { + prefixes <- vapply( + include, + function(id) is.na(id@name["table"]), + logical(1) + ) + if (any(prefixes)) { + cli::cli_abort( + c( + "Namespace selections in {.arg options} are not yet supported.", + i = paste0( + "Select exact tables by adding a {.arg table} component to each ", + "{.fn DBI::Id}; backend namespace expansion will be added separately." + ) + ), + call = call + ) + } + invisible(include) +} diff --git a/R/data-source.R b/R/data-source.R index b49a272..e0effc2 100644 --- a/R/data-source.R +++ b/R/data-source.R @@ -39,6 +39,9 @@ #' #' For a board, a named character vector of pins to read: the names become #' table names, and the values are pin names passed to [pins::pin_read()]. +#' @param options Database selection and future sampling controls from +#' [data_source_options()]. This argument is supported only when `...` is a +#' DBI connection. Supply only one of `tables` and `options`. #' @param dictionary An optional path to a data dictionary describing the #' source's tables and columns, in the #' [data-dict.yaml](https://data-dict.tidyverse.org/) format. See the @@ -85,10 +88,11 @@ #' list_tables(src) #' #' @export -data_source <- function(..., tables = NULL, dictionary = NULL) { +data_source <- function(..., tables = NULL, dictionary = NULL, options = NULL) { dots <- rlang::list2(...) dictionary <- as_data_dictionary(dictionary) kind <- data_source_kind(dots) + options <- check_data_source_options(options, tables, kind) local_commons_span( "commons_data_source_create", @@ -96,7 +100,12 @@ data_source <- function(..., tables = NULL, dictionary = NULL) { ) if (kind == "connection") { - return(data_source_connection(dots[[1]], tables, dictionary = dictionary)) + return(data_source_connection( + dots[[1]], + tables, + dictionary = dictionary, + options = options + )) } if (kind == "board") { return(data_source_board(dots[[1]], tables, dictionary = dictionary)) @@ -139,18 +148,39 @@ data_source_connection <- function( con, tables, dictionary = NULL, + options = NULL, call = rlang::caller_env() ) { span <- local_commons_span("commons_data_source_list_tables") - if (is.null(tables)) { + include <- options$include %||% tables + if (is.null(include)) { listed <- DBI::dbListTables(con) commons_span_set_attribute(span, "commons.data_source.n_tables", length(listed)) - return(new_data_source(con, listed, owned = FALSE, dictionary = dictionary)) + return(new_data_source( + con, + listed, + owned = FALSE, + dictionary = dictionary, + options = options + )) } - table_registry <- normalize_table_registry(tables, call = call) - check_table_ids_exist(con, table_registry, call = call) + if (!is.null(options)) { + check_exact_connection_includes(include, call = call) + } + argument <- if (is.null(options)) "tables" else "include" + table_registry <- normalize_table_registry( + include, + call = call, + argument = argument + ) + check_table_ids_exist( + con, + table_registry, + call = call, + argument = argument + ) commons_span_set_attribute( span, "commons.data_source.n_tables", @@ -162,7 +192,8 @@ data_source_connection <- function( table_registry$labels, owned = FALSE, table_ids = table_registry$ids, - dictionary = dictionary + dictionary = dictionary, + options = options ) } @@ -241,7 +272,8 @@ new_data_source <- function( owned, table_ids = table_ids_from_labels(tables), dictionary = NULL, - pending = NULL + pending = NULL, + options = NULL ) { # Disconnect only the DuckDB connection we created; a user-supplied connection # has its own owner and lifetime. @@ -263,7 +295,8 @@ new_data_source <- function( table_ids = table_ids, handle = handle, dictionary = dictionary, - pending = pending + pending = pending, + options = options ), class = "commons_data_source" ) @@ -561,7 +594,11 @@ duckdb_connect <- function() { con } -normalize_table_registry <- function(tables, call = rlang::caller_env()) { +normalize_table_registry <- function( + tables, + call = rlang::caller_env(), + argument = "tables" +) { entries <- table_entries(tables, call = call) ids <- lapply(entries, table_entry_id, call = call) labels <- vapply(ids, table_id_label, character(1), call = call) @@ -569,7 +606,7 @@ normalize_table_registry <- function(tables, call = rlang::caller_env()) { duplicated_labels <- unique(labels[duplicated(labels)]) if (length(duplicated_labels)) { cli::cli_abort( - "{.arg tables} must not contain duplicate labels: {.val {duplicated_labels}}.", + "{.arg {argument}} must not contain duplicate labels: {.val {duplicated_labels}}.", call = call ) } @@ -661,7 +698,12 @@ table_ids_from_labels <- function(tables) { # startup against a remote warehouse. Probe every table in a single zero-row # query instead, and fall back to per-table checks only to name the missing # tables when that probe fails. -check_table_ids_exist <- function(con, table_registry, call = rlang::caller_env()) { +check_table_ids_exist <- function( + con, + table_registry, + call = rlang::caller_env(), + argument = "tables" +) { probes <- vapply( table_registry$ids, function(id) { @@ -689,14 +731,14 @@ check_table_ids_exist <- function(con, table_registry, call = rlang::caller_env( if (length(missing) == 0) { cli::cli_abort( - "Failed to verify {.arg tables} against the connection.", + "Failed to verify {.arg {argument}} against the connection.", parent = probe_error, call = call ) } cli::cli_abort( - "{.arg tables} names table{?s} not on the connection: {.val {missing}}.", + "{.arg {argument}} names table{?s} not on the connection: {.val {missing}}.", call = call ) } diff --git a/man/data_source.Rd b/man/data_source.Rd index 500ba9e..8b6e77e 100644 --- a/man/data_source.Rd +++ b/man/data_source.Rd @@ -4,7 +4,7 @@ \alias{data_source} \title{Create a data source} \usage{ -data_source(..., tables = NULL, dictionary = NULL) +data_source(..., tables = NULL, dictionary = NULL, options = NULL) } \arguments{ \item{...}{A single DBI connection, a single \code{pins} board, or named data @@ -27,6 +27,10 @@ table names, and the values are pin names passed to \code{\link[pins:pin_read]{p source's tables and columns, in the \href{https://data-dict.tidyverse.org/}{data-dict.yaml} format. See the \verb{Data dictionaries} section.} + +\item{options}{Database selection and future sampling controls from +\code{\link[=data_source_options]{data_source_options()}}. This argument is supported only when \code{...} is a +DBI connection. Supply only one of \code{tables} and \code{options}.} } \value{ A \code{commons_data_source} object. diff --git a/man/data_source_options.Rd b/man/data_source_options.Rd new file mode 100644 index 0000000..2f6cec8 --- /dev/null +++ b/man/data_source_options.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-source-options.R +\name{data_source_options} +\alias{data_source_options} +\title{Configure a database data source} +\usage{ +data_source_options(include = NULL, sample_rows = 0L) +} +\arguments{ +\item{include}{Exact tables to expose: a character vector, a \code{\link[DBI:Id]{DBI::Id()}}, or +a list of \code{DBI::Id()} objects. An ID without a \code{table} component represents +a namespace; namespace expansion requires backend catalog support and is +not yet available.} + +\item{sample_rows}{A non-negative integer recording how many rows to sample +when describing a table. Table descriptions do not yet apply this value.} +} +\value{ +A \code{commons_data_source_options} object for \code{\link[=data_source]{data_source()}}. +} +\description{ +\code{data_source_options()} controls which objects a DBI-backed \code{\link[=data_source]{data_source()}} +exposes. Character values select literal table names. Use \code{\link[DBI:Id]{DBI::Id()}} to +preserve catalog and schema components or to select a name containing dots. +} diff --git a/tests/testthat/README.md b/tests/testthat/README.md index c510119..cad5fb0 100644 --- a/tests/testthat/README.md +++ b/tests/testthat/README.md @@ -26,6 +26,7 @@ Set `COMMONS_DATABRICKS_DSN` to use a differently named DSN. Identifiers are passed as separate `catalog`, `schema`, and `table` components of `DBI::Id()`; do not combine them into a dotted string. -Each test queries the current identity and namespace, then reads at most one row -from the configured table. If a live-test switch or identifier is absent, that -backend's test skips. Once enabled, connection and query failures fail the test. +Each test queries the current identity and namespace, constructs a data source +from the structured table identifier, then describes at most one row from that +table. If a live-test switch or identifier is absent, that backend's test skips. +Once enabled, connection and query failures fail the test. diff --git a/tests/testthat/helper-live-warehouses.R b/tests/testthat/helper-live-warehouses.R index 47b0ab5..083ac12 100644 --- a/tests/testthat/helper-live-warehouses.R +++ b/tests/testthat/helper-live-warehouses.R @@ -44,12 +44,3 @@ skip_unless_live_warehouse <- function(backend) { paste0("Set ", variable, "=true to run live warehouse tests") ) } - -warehouse_read_one <- function(con, id) { - sql <- paste( - "SELECT * FROM", - DBI::dbQuoteIdentifier(con, id), - "LIMIT 1" - ) - DBI::dbGetQuery(con, sql) -} diff --git a/tests/testthat/test-data-source-options.R b/tests/testthat/test-data-source-options.R new file mode 100644 index 0000000..6809ad9 --- /dev/null +++ b/tests/testthat/test-data-source-options.R @@ -0,0 +1,116 @@ +test_that("data_source_options normalizes exact selectors", { + options <- data_source_options( + include = c("sales", "crm.sales"), + sample_rows = 2 + ) + + expect_s3_class(options, "commons_data_source_options") + expect_identical( + lapply(options$include, function(id) id@name), + list(c(table = "sales"), c(table = "crm.sales")) + ) + expect_identical(options$sample_rows, 2L) + + qualified <- DBI::Id(catalog = "analytics", schema = "crm", table = "sales") + expect_identical(data_source_options(qualified)$include[[1]], qualified) +}) + +test_that("data_source_options validates its values", { + expect_error(data_source_options(include = ""), "non-empty table names") + expect_error( + data_source_options(include = NA_character_), + "non-empty table names" + ) + expect_error( + data_source_options(include = character()), + "at least one object" + ) + expect_error(data_source_options(include = 1), "must be a character vector") + expect_error( + data_source_options(include = list("sales")), + "Every entry.*DBI::Id" + ) + expect_error( + data_source_options(include = DBI::Id(schema = "")), + "non-empty components" + ) + + invalid_rows <- list(-1, 1.5, NA_real_, Inf, "1", c(1, 2)) + for (sample_rows in invalid_rows) { + expect_error( + data_source_options(sample_rows = sample_rows), + "one non-negative integer" + ) + } +}) + +test_that("options character selectors are literal table names", { + con <- DBI::dbConnect(duckdb::duckdb()) + withr::defer(DBI::dbDisconnect(con, shutdown = TRUE)) + DBI::dbExecute(con, 'CREATE TABLE "crm.sales" (order_id VARCHAR)') + DBI::dbExecute(con, 'INSERT INTO "crm.sales" VALUES (\'o01\')') + + src <- data_source( + con, + options = data_source_options(include = "crm.sales", sample_rows = 1L) + ) + + expect_equal(list_tables(src), "crm.sales") + expect_identical(src$table_ids[["crm.sales"]]@name, c(table = "crm.sales")) + expect_identical(src$options$sample_rows, 1L) + expect_equal( + source_describe(src, "crm.sales", n_sample = 1)$sample$order_id, + "o01" + ) +}) + +test_that("options preserve and quote structured identifiers", { + con <- DBI::dbConnect(duckdb::duckdb()) + withr::defer(DBI::dbDisconnect(con, shutdown = TRUE)) + DBI::dbExecute(con, 'CREATE SCHEMA "crm raw"') + id <- DBI::Id(schema = "crm raw", table = "sales.daily") + quoted <- DBI::dbQuoteIdentifier(con, id) + DBI::dbExecute(con, paste("CREATE TABLE", quoted, "(order_id VARCHAR)")) + DBI::dbExecute(con, paste("INSERT INTO", quoted, "VALUES ('o01')")) + + src <- data_source(con, options = data_source_options(include = id)) + + expect_equal(list_tables(src), "crm raw.sales.daily") + expect_identical(src$table_ids[["crm raw.sales.daily"]], id) + expect_equal( + source_describe(src, "crm raw.sales.daily", n_sample = 1)$sample$order_id, + "o01" + ) +}) + +test_that("data_source options are confined to DBI connections", { + con <- DBI::dbConnect(duckdb::duckdb()) + withr::defer(DBI::dbDisconnect(con, shutdown = TRUE)) + DBI::dbWriteTable(con, "sales", data.frame(id = 1L)) + options <- data_source_options(include = "sales") + + expect_error( + data_source(con, tables = "sales", options = options), + "only one of `tables` and `options`" + ) + expect_error( + data_source(con, options = list(include = "sales")), + "must come from" + ) + expect_error( + data_source(sales = data.frame(id = 1L), options = options), + "only be used with a DBI connection" + ) +}) + +test_that("namespace options stop at the backend expansion boundary", { + options <- data_source_options(include = DBI::Id(schema = "crm")) + expect_identical(options$include[[1]], DBI::Id(schema = "crm")) + + con <- DBI::dbConnect(duckdb::duckdb()) + withr::defer(DBI::dbDisconnect(con, shutdown = TRUE)) + expect_error( + data_source(con, options = options), + "Namespace selections in `options` are not yet supported" + ) +}) diff --git a/tests/testthat/test-live-warehouses.R b/tests/testthat/test-live-warehouses.R index dae3ecc..a51fecc 100644 --- a/tests/testthat/test-live-warehouses.R +++ b/tests/testthat/test-live-warehouses.R @@ -11,9 +11,15 @@ test_that("live Snowflake connection reads a configured table", { "CURRENT_SCHEMA() AS schema" ) ) - rows <- warehouse_read_one(con, objects$table) + source <- data_source( + con, + options = data_source_options(include = objects$table, sample_rows = 1L) + ) + label <- paste(objects$table@name, collapse = ".") + rows <- source_describe(source, label, n_sample = 1)$sample names(session) <- tolower(names(session)) + expect_identical(source$table_ids[[label]], objects$table) expect_equal(nrow(session), 1) expect_named(session, c("principal", "role", "catalog", "schema")) expect_true(nzchar(session$principal[[1]])) @@ -33,9 +39,15 @@ test_that("live Databricks connection reads a configured table", { "CURRENT_SCHEMA() AS schema" ) ) - rows <- warehouse_read_one(con, objects$table) + source <- data_source( + con, + options = data_source_options(include = objects$table, sample_rows = 1L) + ) + label <- paste(objects$table@name, collapse = ".") + rows <- source_describe(source, label, n_sample = 1)$sample names(session) <- tolower(names(session)) + expect_identical(source$table_ids[[label]], objects$table) expect_equal(nrow(session), 1) expect_named(session, c("principal", "catalog", "schema")) expect_true(nzchar(session$principal[[1]])) From 4f5c7c17f2f0fc35d6bc96274d16881c8a397d7f Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 13:23:40 -0500 Subject: [PATCH 3/3] remove unused sampling option --- R/data-source-options.R | 27 ++--------------------- R/data-source.R | 4 ++-- man/data_source.Rd | 4 ++-- man/data_source_options.Rd | 5 +---- tests/testthat/test-data-source-options.R | 18 ++------------- 5 files changed, 9 insertions(+), 49 deletions(-) diff --git a/R/data-source-options.R b/R/data-source-options.R index 51edb6d..ce90261 100644 --- a/R/data-source-options.R +++ b/R/data-source-options.R @@ -8,17 +8,11 @@ #' a list of `DBI::Id()` objects. An ID without a `table` component represents #' a namespace; namespace expansion requires backend catalog support and is #' not yet available. -#' @param sample_rows A non-negative integer recording how many rows to sample -#' when describing a table. Table descriptions do not yet apply this value. -#' #' @return A `commons_data_source_options` object for [data_source()]. #' @export -data_source_options <- function(include = NULL, sample_rows = 0L) { +data_source_options <- function(include = NULL) { structure( - list( - include = normalize_connection_includes(include), - sample_rows = check_sample_rows(sample_rows) - ), + list(include = normalize_connection_includes(include)), class = "commons_data_source_options" ) } @@ -109,23 +103,6 @@ normalize_connection_includes <- function( entries } -check_sample_rows <- function(sample_rows, call = rlang::caller_env()) { - valid <- is.numeric(sample_rows) && - length(sample_rows) == 1 && - !is.na(sample_rows) && - is.finite(sample_rows) && - sample_rows >= 0 && - sample_rows <= .Machine$integer.max && - sample_rows == as.integer(sample_rows) - if (!valid) { - cli::cli_abort( - "{.arg sample_rows} must be one non-negative integer.", - call = call - ) - } - as.integer(sample_rows) -} - check_exact_connection_includes <- function( include, call = rlang::caller_env() diff --git a/R/data-source.R b/R/data-source.R index e0effc2..671c4ca 100644 --- a/R/data-source.R +++ b/R/data-source.R @@ -39,8 +39,8 @@ #' #' For a board, a named character vector of pins to read: the names become #' table names, and the values are pin names passed to [pins::pin_read()]. -#' @param options Database selection and future sampling controls from -#' [data_source_options()]. This argument is supported only when `...` is a +#' @param options Database selection controls from [data_source_options()]. +#' This argument is supported only when `...` is a #' DBI connection. Supply only one of `tables` and `options`. #' @param dictionary An optional path to a data dictionary describing the #' source's tables and columns, in the diff --git a/man/data_source.Rd b/man/data_source.Rd index 8b6e77e..ce15354 100644 --- a/man/data_source.Rd +++ b/man/data_source.Rd @@ -28,8 +28,8 @@ source's tables and columns, in the \href{https://data-dict.tidyverse.org/}{data-dict.yaml} format. See the \verb{Data dictionaries} section.} -\item{options}{Database selection and future sampling controls from -\code{\link[=data_source_options]{data_source_options()}}. This argument is supported only when \code{...} is a +\item{options}{Database selection controls from \code{\link[=data_source_options]{data_source_options()}}. +This argument is supported only when \code{...} is a DBI connection. Supply only one of \code{tables} and \code{options}.} } \value{ diff --git a/man/data_source_options.Rd b/man/data_source_options.Rd index 2f6cec8..f81c698 100644 --- a/man/data_source_options.Rd +++ b/man/data_source_options.Rd @@ -4,16 +4,13 @@ \alias{data_source_options} \title{Configure a database data source} \usage{ -data_source_options(include = NULL, sample_rows = 0L) +data_source_options(include = NULL) } \arguments{ \item{include}{Exact tables to expose: a character vector, a \code{\link[DBI:Id]{DBI::Id()}}, or a list of \code{DBI::Id()} objects. An ID without a \code{table} component represents a namespace; namespace expansion requires backend catalog support and is not yet available.} - -\item{sample_rows}{A non-negative integer recording how many rows to sample -when describing a table. Table descriptions do not yet apply this value.} } \value{ A \code{commons_data_source_options} object for \code{\link[=data_source]{data_source()}}. diff --git a/tests/testthat/test-data-source-options.R b/tests/testthat/test-data-source-options.R index 6809ad9..0579e2a 100644 --- a/tests/testthat/test-data-source-options.R +++ b/tests/testthat/test-data-source-options.R @@ -1,16 +1,11 @@ test_that("data_source_options normalizes exact selectors", { - options <- data_source_options( - include = c("sales", "crm.sales"), - sample_rows = 2 - ) + options <- data_source_options(include = c("sales", "crm.sales")) expect_s3_class(options, "commons_data_source_options") expect_identical( lapply(options$include, function(id) id@name), list(c(table = "sales"), c(table = "crm.sales")) ) - expect_identical(options$sample_rows, 2L) - qualified <- DBI::Id(catalog = "analytics", schema = "crm", table = "sales") expect_identical(data_source_options(qualified)$include[[1]], qualified) }) @@ -34,14 +29,6 @@ test_that("data_source_options validates its values", { data_source_options(include = DBI::Id(schema = "")), "non-empty components" ) - - invalid_rows <- list(-1, 1.5, NA_real_, Inf, "1", c(1, 2)) - for (sample_rows in invalid_rows) { - expect_error( - data_source_options(sample_rows = sample_rows), - "one non-negative integer" - ) - } }) test_that("options character selectors are literal table names", { @@ -52,12 +39,11 @@ test_that("options character selectors are literal table names", { src <- data_source( con, - options = data_source_options(include = "crm.sales", sample_rows = 1L) + options = data_source_options(include = "crm.sales") ) expect_equal(list_tables(src), "crm.sales") expect_identical(src$table_ids[["crm.sales"]]@name, c(table = "crm.sales")) - expect_identical(src$options$sample_rows, 1L) expect_equal( source_describe(src, "crm.sales", n_sample = 1)$sample$order_id, "o01"