From 1fea27aa65b9513aa7bf9fb5b32fde57e180fba8 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed Date: Wed, 4 Nov 2020 15:23:26 +1000 Subject: [PATCH 1/2] Update `grepl` SQL translation to match REGEXP_CONTAINS signature `grepl` function in base R is incorrectly translated to `REGEXP_CONTAINS` by direct replacement. However, both reverse arguments order: - `grepl(pattern, x)` - `REGEXP_CONTAINS(value, regexp)` This PR fixes the order of arguments for `REGEXP_CONTAINS`. --- R/dplyr.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/dplyr.R b/R/dplyr.R index dc82bc45..182c7c0a 100644 --- a/R/dplyr.R +++ b/R/dplyr.R @@ -189,7 +189,9 @@ sql_translate_env.BigQueryConnection <- function(x) { Sys.time = sql_prefix("current_time"), # Regular expressions - grepl = sql_prefix("REGEXP_CONTAINS", 2), + grepl = function(pattern, x) { + dbplyr::build_sql("REGEXP_CONTAINS", list(x, pattern)) + }, gsub = function(match, replace, x) { dbplyr::build_sql("REGEXP_REPLACE", list(x, match, replace)) }, From 72a615f8cdb3ffd34b7d9bf381439224e2aa34a2 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Nov 2023 08:53:01 -0600 Subject: [PATCH 2/2] Add test & news bullet --- NEWS.md | 3 +++ R/dplyr.R | 6 ++++-- tests/testthat/_snaps/dplyr.md | 11 +++++++++++ tests/testthat/test-dplyr.R | 9 +++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 4f09ec29..f73bd3eb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # bigrquery (development version) +* `grepl(pattern, x)` is now correctly translated to + `REGEXP_CONTAINS(x, pattern)` (#416). + * `median()` gets a translation that works in `summarise()` and a clear error if you use it in `mutate()` (#419). diff --git a/R/dplyr.R b/R/dplyr.R index b56592ec..304e648f 100644 --- a/R/dplyr.R +++ b/R/dplyr.R @@ -233,10 +233,12 @@ sql_translation.BigQueryConnection <- function(x) { # Regular expressions grepl = function(pattern, x) { + # https://cloud.google.com/bigquery/docs/reference/standard-sql/string_functions#regexp_contains dbplyr::build_sql("REGEXP_CONTAINS", list(x, pattern)) }, - gsub = function(match, replace, x) { - dbplyr::build_sql("REGEXP_REPLACE", list(x, match, replace)) + gsub = function(pattern, replace, x) { + # https://cloud.google.com/bigquery/docs/reference/standard-sql/string_functions#regexp_replace + dbplyr::build_sql("REGEXP_REPLACE", list(x, pattern, replace)) }, # Other scalar functions diff --git a/tests/testthat/_snaps/dplyr.md b/tests/testthat/_snaps/dplyr.md index 936bcc1f..7e46b1ad 100644 --- a/tests/testthat/_snaps/dplyr.md +++ b/tests/testthat/_snaps/dplyr.md @@ -6,3 +6,14 @@ Error in `db_copy_to()`: ! BigQuery does not support temporary tables +# string functions correctly + + Code + dbplyr::translate_sql(grepl("a.c", x), con = con) + Output + REGEXP_CONTAINS(`x`, 'a.c') + Code + dbplyr::translate_sql(gsub("a.c", "", x), con = con) + Output + REGEXP_REPLACE(`x`, 'a.c', '') + diff --git a/tests/testthat/test-dplyr.R b/tests/testthat/test-dplyr.R index 7f6bbd38..8cd2e1be 100644 --- a/tests/testthat/test-dplyr.R +++ b/tests/testthat/test-dplyr.R @@ -137,6 +137,15 @@ test_that("runif is correctly translated", { ) }) +test_that("string functions correctly", { + con <- simulate_bigrquery() + + expect_snapshot({ + dbplyr::translate_sql(grepl("a.c", x), con = con) + dbplyr::translate_sql(gsub("a.c", "", x), con = con) + }) +}) + test_that("median is correctly translated", { expect_equal( dbplyr::translate_sql(median(x), con = simulate_bigrquery(), window = FALSE),