From 791608fb52d6429e3e8905a6a9e36e5582b5c6a7 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sat, 10 Apr 2021 23:39:07 +0100 Subject: [PATCH 01/10] Attempt to add parameter support for DBI calls --- R/bq-perform.R | 2 +- R/dbi-result.R | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/R/bq-perform.R b/R/bq-perform.R index 77f48b2e..8938530e 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -259,7 +259,7 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) - if (!is.null(parameters)) { + if (!is.null(parameters) & !(length(parameters) == 0)) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) } diff --git a/R/dbi-result.R b/R/dbi-result.R index 42608397..ecaaec5c 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -1,12 +1,22 @@ #' @include dbi-connection.R NULL -BigQueryResult <- function(conn, sql) { +BigQueryResult <- function(conn, sql, ...) { + + args <- c(as.list(environment()), list(...)) + if ("params" %in% names(args)) { + params = args["params"] + } else { + params = list() + } + if (is.null(conn@dataset)) { - tb <- bq_project_query(conn@billing, sql, quiet = conn@quiet) + tb <- bq_project_query(conn@billing, sql, quiet = conn@quiet, parameters = params) } else { ds <- as_bq_dataset(conn) - tb <- bq_dataset_query(ds, sql, quiet = conn@quiet, billing = conn@billing) + tb <- bq_dataset_query( + ds, sql, quiet = conn@quiet, billing = conn@billing, parameters = params + ) } nrow <- bq_table_nrow(tb) From 5d0e3eb3979e347843003c231e37626cf6fd23b4 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 01:21:17 +0100 Subject: [PATCH 02/10] Added basic test for the DBI interface using parameters --- tests/testthat/test-dbi-result.R | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/testthat/test-dbi-result.R b/tests/testthat/test-dbi-result.R index cf53d9f6..569af81d 100644 --- a/tests/testthat/test-dbi-result.R +++ b/tests/testthat/test-dbi-result.R @@ -10,6 +10,19 @@ test_that("can retrieve full query results", { expect_equal(df, tibble(count = 32L)) }) +test_that("can retrieve parameterized query results", { + con <- DBI::dbConnect( + bigquery(), + project = bq_test_project(), + dataset = "basedata", + bigint = "integer" + ) + + df <- DBI::dbGetQuery(con, "SELECT @value AS value", params = (value = 100)) + expect_equal(df, tibble(value = 100)) +}) + + test_that("can retrieve without dataset", { con <- DBI::dbConnect( bigquery(), From 17e06026e5443f63ee15106263f6d3e0c79ccd8e Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 01:59:26 +0100 Subject: [PATCH 03/10] Comment + swap parameter order --- R/dbi-result.R | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/R/dbi-result.R b/R/dbi-result.R index 371af2c2..9a9e065b 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -3,6 +3,8 @@ NULL BigQueryResult <- function(conn, sql, ...) { + # extract params of the query if provided + # to support DBI interface for parameterized queries args <- c(as.list(environment()), list(...)) if ("params" %in% names(args)) { params = args["params"] @@ -10,13 +12,12 @@ BigQueryResult <- function(conn, sql, ...) { params = list() } - if (is.null(conn@dataset)) { job <- bq_perform_query(sql, billing = conn@billing, quiet = conn@quiet, - ..., - parameters = params + parameters = params, + ... ) } else { ds <- as_bq_dataset(conn) @@ -24,8 +25,8 @@ BigQueryResult <- function(conn, sql, ...) { billing = conn@billing, default_dataset = ds, quiet = conn@quiet, - ..., - parameters = params + parameters = params, + ... ) } From ddfbe1a00f0877240d3d0c690bf337f20a76f0a7 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 18:20:10 +0100 Subject: [PATCH 04/10] debugging parameters --- R/dbi-result.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/dbi-result.R b/R/dbi-result.R index 9a9e065b..a362cc50 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -12,6 +12,7 @@ BigQueryResult <- function(conn, sql, ...) { params = list() } + message(params) if (is.null(conn@dataset)) { job <- bq_perform_query(sql, billing = conn@billing, From 2f1cab19b57ed079230e8665d542d4cefd4f39e4 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 18:29:17 +0100 Subject: [PATCH 05/10] Debug parameters that perform query gets --- R/bq-perform.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/bq-perform.R b/R/bq-perform.R index 7ed89262..62dc17d3 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -263,6 +263,7 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) + message("Parameters in bq_perfrom_query: ", parameters) if (!is.null(parameters) & !(length(parameters) == 0)) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) From cae095d9ed91238e1540850a3993f52b0f6fe214 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 18:37:27 +0100 Subject: [PATCH 06/10] Change dubug to print --- R/bq-perform.R | 3 ++- R/dbi-result.R | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/R/bq-perform.R b/R/bq-perform.R index 62dc17d3..c2a31662 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -263,7 +263,8 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) - message("Parameters in bq_perfrom_query: ", parameters) + print("Parameters in bq_perfrom_query: /n") + print(parameters) if (!is.null(parameters) & !(length(parameters) == 0)) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) diff --git a/R/dbi-result.R b/R/dbi-result.R index a362cc50..2cbd703f 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -12,7 +12,7 @@ BigQueryResult <- function(conn, sql, ...) { params = list() } - message(params) + print(params) if (is.null(conn@dataset)) { job <- bq_perform_query(sql, billing = conn@billing, From e8474470a3e2add92ebdb42619c56fcfe14c18df Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 11 Apr 2021 18:40:10 +0100 Subject: [PATCH 07/10] Fixes element access and removes debug --- R/bq-perform.R | 2 -- R/dbi-result.R | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/R/bq-perform.R b/R/bq-perform.R index c2a31662..7ed89262 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -263,8 +263,6 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) - print("Parameters in bq_perfrom_query: /n") - print(parameters) if (!is.null(parameters) & !(length(parameters) == 0)) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) diff --git a/R/dbi-result.R b/R/dbi-result.R index 2cbd703f..dcac113b 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -7,12 +7,11 @@ BigQueryResult <- function(conn, sql, ...) { # to support DBI interface for parameterized queries args <- c(as.list(environment()), list(...)) if ("params" %in% names(args)) { - params = args["params"] + params = args[["params"]] } else { params = list() } - print(params) if (is.null(conn@dataset)) { job <- bq_perform_query(sql, billing = conn@billing, From bf46d0b8e33a4bf604e33284fbc844bbe8123d03 Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 18 Apr 2021 22:55:44 +0100 Subject: [PATCH 08/10] Updated news --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 62510cc0..3db6f465 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ * Improve int64 support when reading BigQuery tables with dplyr syntax. `collect()` now utilizes `bigint` parameter in `DBI::dbConnect()` object. Set `bigint` in connection object to `"integer64"` or `"character"` to avoid integer coercion and overflow issues (@zoews, #439, #437). +* `DBI:dbGetQuery()` - now can take params attribute which will work with parameterised queries (@byapparov, #443) + # bigrquery 1.3.2 * BigQuery `BYTES` and `GEOGRAPHY` column types are now supported via From 9dc98018b4ef3f08ff1297a2ce3449c3b9efd7fc Mon Sep 17 00:00:00 2001 From: byapparov Date: Sun, 18 Apr 2021 22:58:19 +0100 Subject: [PATCH 09/10] Reduce and correct the if statement --- R/bq-perform.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/bq-perform.R b/R/bq-perform.R index 7ed89262..63753c40 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -263,7 +263,7 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) - if (!is.null(parameters) & !(length(parameters) == 0)) { + if (!is.null(parameters) && length(parameters) > 0) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) } From 97afb455ebe0b806c2316455a672c2252630523b Mon Sep 17 00:00:00 2001 From: byapparov Date: Fri, 21 May 2021 20:33:21 +0100 Subject: [PATCH 10/10] Response to the code review --- R/bq-perform.R | 2 +- R/dbi-result.R | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/R/bq-perform.R b/R/bq-perform.R index 63753c40..3ba56da1 100644 --- a/R/bq-perform.R +++ b/R/bq-perform.R @@ -263,7 +263,7 @@ bq_perform_query <- function(query, billing, priority = unbox(priority) ) - if (!is.null(parameters) && length(parameters) > 0) { + if (length(parameters) > 0) { parameters <- as_bq_params(parameters) query$queryParameters <- as_json(parameters) } diff --git a/R/dbi-result.R b/R/dbi-result.R index dcac113b..8ba44bad 100644 --- a/R/dbi-result.R +++ b/R/dbi-result.R @@ -1,16 +1,7 @@ #' @include dbi-connection.R NULL -BigQueryResult <- function(conn, sql, ...) { - - # extract params of the query if provided - # to support DBI interface for parameterized queries - args <- c(as.list(environment()), list(...)) - if ("params" %in% names(args)) { - params = args[["params"]] - } else { - params = list() - } +BigQueryResult <- function(conn, sql, params = NULL, ...) { if (is.null(conn@dataset)) { job <- bq_perform_query(sql,