fix(r): cast a Snowflake semantic view variable to its declared type

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).

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 {
       ""

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())

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))) {

pkg-r/tests/testthat/test-catalog-snowflake.R

@@ -421,7 +421,43 @@ 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)'
+  )
+
+  # 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(), odd, "SINCE"),
+    'VARIABLES "SINCE" => ?'
+  )
 })
 
 test_that("Snowflake hydrates only explicitly selected semantic views", {