From 0763f1ec87b167d4ddd0224ecc54f095f6ffed3c Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 21 Oct 2026 20:32:20 -0600 Subject: [PATCH 1/2] fix(r): cast a Snowflake semantic view variable to its declared type Snowflake requires a variable's value to match the type the view declares, and Snowflake ODBC driver 4.x binds a number as text, so a parameterized metric call failed. The declared type is now kept when the view is read, and the metric query and the access probe both write CAST(? AS ). A declared type that is not a plain type name is not spliced into SQL, and that variable keeps a bare placeholder. Closes #447 --- pkg-r/NEWS.md | 2 ++ pkg-r/R/catalog-security.R | 10 ++----- pkg-r/R/catalog-snowflake.R | 27 +++++++++++------- pkg-r/R/semantic-models.R | 12 +++++++- pkg-r/tests/testthat/test-catalog-snowflake.R | 28 ++++++++++++++++++- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/pkg-r/NEWS.md b/pkg-r/NEWS.md index 02d1e891..0e925be4 100644 --- a/pkg-r/NEWS.md +++ b/pkg-r/NEWS.md @@ -1,5 +1,7 @@ # commons (development version) +* A Snowflake semantic view's variables are now bound with the type the view declares, such as `NUMBER(10,2)`. Snowflake refuses a value of another type, and an ODBC driver can bind a number as text (#447). + * A Snowflake semantic view whose base tables are named with quoted identifiers, such as `"orders"`, now scopes to those tables. The quotes were kept as part of each name, so `data_source()` could not find them (#446). * A definition that adds `NULL` to a date, such as `created + NULL`, now compiles to SQL that Snowflake accepts. It compiles to a typed `NULL` timestamp (#441). diff --git a/pkg-r/R/catalog-security.R b/pkg-r/R/catalog-security.R index 0abbbf34..6088cbcc 100644 --- a/pkg-r/R/catalog-security.R +++ b/pkg-r/R/catalog-security.R @@ -475,14 +475,8 @@ catalog_semantic_probe_sql <- function(con, model, arguments = list()) { reference <- catalog_semantic_member_reference(con, member) variables <- if (length(arguments)) { paste0( - "\n VARIABLES ", - paste( - sprintf( - "%s => ?", - DBI::dbQuoteIdentifier(con, names(arguments)) - ), - collapse = ", " - ) + "\n ", + snowflake_semantic_variables(con, model, names(arguments)) ) } else { "" diff --git a/pkg-r/R/catalog-snowflake.R b/pkg-r/R/catalog-snowflake.R index 3f2a44bf..551bc6c1 100644 --- a/pkg-r/R/catalog-snowflake.R +++ b/pkg-r/R/catalog-snowflake.R @@ -802,16 +802,7 @@ snowflake_semantic_metric_sql <- function( paste(snowflake_semantic_member_references(metrics, con), collapse = ", ") ), if (length(arguments)) { - paste( - "VARIABLES", - paste( - sprintf( - "%s => ?", - DBI::dbQuoteIdentifier(con, names(arguments)) - ), - collapse = ", " - ) - ) + snowflake_semantic_variables(con, model, names(arguments)) } ) conditions <- c( @@ -828,6 +819,22 @@ snowflake_semantic_metric_sql <- function( ) } +snowflake_semantic_variables <- function(con, model, names) { + placeholders <- vapply(names, function(name) { + type <- model$parameters[[name]]$sql_type + if (is.null(type)) "?" else sprintf("CAST(? AS %s)", type) + }, character(1)) + paste( + "VARIABLES", + paste( + DBI::dbQuoteIdentifier(con, names), + "=>", + placeholders, + collapse = ", " + ) + ) +} + snowflake_semantic_member_references <- function(members, con) { if (is.null(members) || nrow(members) == 0L) { return(character()) diff --git a/pkg-r/R/semantic-models.R b/pkg-r/R/semantic-models.R index a89170ff..252fcf89 100644 --- a/pkg-r/R/semantic-models.R +++ b/pkg-r/R/semantic-models.R @@ -114,13 +114,23 @@ semantic_parameters_from_spec <- function(entries, backend) { default <- if (has_default) { coerce_semantic_default(entry[[default_field]], type) } - new_typed_argument( + argument <- new_typed_argument( name, type, description = entry$description %||% entry$comment, default = default, has_default = has_default ) + # Snowflake requires a variable's value to match its declared type, so + # the type is kept for the query to cast to. It is spliced into SQL, so + # only a plain type name is kept. + if ( + identical(backend, "Snowflake") && + grepl("^[A-Za-z_]+( *\\( *[0-9]+( *, *[0-9]+)? *\\))?$", declared_type) + ) { + argument$sql_type <- toupper(gsub(" ", "", declared_type, fixed = TRUE)) + } + argument }) names(parameters) <- vapply(parameters, `[[`, character(1), "name") if (anyDuplicated(names(parameters))) { diff --git a/pkg-r/tests/testthat/test-catalog-snowflake.R b/pkg-r/tests/testthat/test-catalog-snowflake.R index fb2184be..e68cdd6a 100644 --- a/pkg-r/tests/testthat/test-catalog-snowflake.R +++ b/pkg-r/tests/testthat/test-catalog-snowflake.R @@ -421,7 +421,33 @@ test_that("Snowflake imports and binds semantic variables", { expect_equal(model$parameters$threshold$default, 42) expect_false(model$parameters$category$has_default) expect_identical(model$parameters$large_id$default, 2147483648) - expect_match(sql, 'VARIABLES "category" => ?', fixed = TRUE) + expect_match(sql, 'VARIABLES "category" => CAST(? AS TEXT)', fixed = TRUE) +}) + +test_that("Snowflake casts a bound variable to the type its view declares", { + # Snowflake requires a variable's value to match the declared type, and a + # driver may bind a number as text, so the placeholder is cast. + model <- snowflake_semantic_model_from_spec( + DBI::Id(catalog = "ANALYTICS", schema = "PUBLIC", table = "MODEL"), + list( + variables = list( + list(name = "MIN_TOTAL", data_type = "NUMBER(10,2)"), + list(name = "SINCE", data_type = "TIMESTAMP_TZ") + ), + metrics = list(list(name = "revenue")) + ) + ) + + expect_equal( + snowflake_semantic_variables(DBI::ANSI(), model, c("MIN_TOTAL", "SINCE")), + 'VARIABLES "MIN_TOTAL" => CAST(? AS NUMBER(10,2)), "SINCE" => CAST(? AS TIMESTAMP_TZ)' + ) + + model$parameters$SINCE$sql_type <- NULL + expect_equal( + snowflake_semantic_variables(DBI::ANSI(), model, "SINCE"), + 'VARIABLES "SINCE" => ?' + ) }) test_that("Snowflake hydrates only explicitly selected semantic views", { From a990154b8ae18743cddae3d5f4d35db834cff54d Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 21 Oct 2026 20:35:32 -0600 Subject: [PATCH 2/2] test(r): read a non-plain variable type from the view's specification --- pkg-r/tests/testthat/test-catalog-snowflake.R | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg-r/tests/testthat/test-catalog-snowflake.R b/pkg-r/tests/testthat/test-catalog-snowflake.R index e68cdd6a..135dcf82 100644 --- a/pkg-r/tests/testthat/test-catalog-snowflake.R +++ b/pkg-r/tests/testthat/test-catalog-snowflake.R @@ -443,9 +443,19 @@ test_that("Snowflake casts a bound variable to the type its view declares", { 'VARIABLES "MIN_TOTAL" => CAST(? AS NUMBER(10,2)), "SINCE" => CAST(? AS TIMESTAMP_TZ)' ) - model$parameters$SINCE$sql_type <- NULL + # A declared type is spliced into SQL, so one that is not a plain type name + # leaves its variable with a bare placeholder. + odd <- snowflake_semantic_model_from_spec( + DBI::Id(catalog = "ANALYTICS", schema = "PUBLIC", table = "MODEL"), + list( + variables = list( + list(name = "SINCE", data_type = "TIMESTAMP_TZ); DROP TABLE x; --") + ), + metrics = list(list(name = "revenue")) + ) + ) expect_equal( - snowflake_semantic_variables(DBI::ANSI(), model, "SINCE"), + snowflake_semantic_variables(DBI::ANSI(), odd, "SINCE"), 'VARIABLES "SINCE" => ?' ) })