From d22e8e9009cbe46fcb80b477730dda91ed4f9369 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 4 Apr 2024 13:53:12 +0100 Subject: [PATCH 01/12] str_like case sensitivity If ignore_case is true, an error is thrown for dbms without ILIKE. If ignore_case is false, LIKE is used consistently across dbms --- R/backend-.R | 9 +++++++-- tests/testthat/_snaps/backend-.md | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/R/backend-.R b/R/backend-.R index 3557e252d..862d41bd8 100644 --- a/R/backend-.R +++ b/R/backend-.R @@ -282,9 +282,14 @@ base_scalar <- sql_translator( str_sub = sql_str_sub("SUBSTR"), str_like = function(string, pattern, ignore_case = TRUE) { if (isTRUE(ignore_case)) { - sql_expr(!!string %LIKE% !!pattern) + bullets <- c( + "Backend does not support case insensitve {.fn str_like}.", + i = "Set ignore_case = FALSE for case sensitive match.", + i = "Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match." + ) + abort(bullets) } else { - cli::cli_abort("Backend only supports case insensitve {.fn str_like}.") + sql_expr(!!string %LIKE% !!pattern) } }, diff --git a/tests/testthat/_snaps/backend-.md b/tests/testthat/_snaps/backend-.md index b2e003bc5..5b414ab16 100644 --- a/tests/testthat/_snaps/backend-.md +++ b/tests/testthat/_snaps/backend-.md @@ -42,7 +42,7 @@ # can translate case insensitive like Code - test_translate_sql(str_like(x, "abc", ignore_case = FALSE)) + test_translate_sql(str_like(x, "abc", ignore_case = TRUE)) Condition Error in `str_like()`: ! Backend only supports case insensitve `str_like()`. From e4da507805579c1cfb84e3bd8c2c3cc3be040dec Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 4 Apr 2024 13:55:45 +0100 Subject: [PATCH 02/12] snapshot test --- tests/testthat/_snaps/backend-.md | 4 +++- tests/testthat/test-backend-.R | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/testthat/_snaps/backend-.md b/tests/testthat/_snaps/backend-.md index 5b414ab16..358ebd8b1 100644 --- a/tests/testthat/_snaps/backend-.md +++ b/tests/testthat/_snaps/backend-.md @@ -45,7 +45,9 @@ test_translate_sql(str_like(x, "abc", ignore_case = TRUE)) Condition Error in `str_like()`: - ! Backend only supports case insensitve `str_like()`. + ! Backend does not support case insensitve {.fn str_like}. + i Set ignore_case = FALSE for case sensitive match. + i Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match. # default raw escapes translated correctly diff --git a/tests/testthat/test-backend-.R b/tests/testthat/test-backend-.R index d04ec01db..741305e2e 100644 --- a/tests/testthat/test-backend-.R +++ b/tests/testthat/test-backend-.R @@ -103,9 +103,9 @@ test_that("lead and lag translate n to integers", { test_that("can translate case insensitive like", { local_con(simulate_dbi()) - test_translate_sql(str_like(x, "abc")) + test_translate_sql(str_like(x, "abc", ignore_case = FALSE)) expect_snapshot( - test_translate_sql(str_like(x, "abc", ignore_case = FALSE)), + test_translate_sql(str_like(x, "abc", ignore_case = TRUE)), error = TRUE ) }) From e6ea1e3a35b1b221b3241d1f37096fc6619ba4ae Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 4 Apr 2024 15:17:42 +0100 Subject: [PATCH 03/12] cli message and snaps --- R/backend-.R | 5 ++--- tests/testthat/_snaps/backend-.md | 2 +- tests/testthat/_snaps/tbl-sql.md | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/R/backend-.R b/R/backend-.R index 862d41bd8..b3c2f845e 100644 --- a/R/backend-.R +++ b/R/backend-.R @@ -282,12 +282,11 @@ base_scalar <- sql_translator( str_sub = sql_str_sub("SUBSTR"), str_like = function(string, pattern, ignore_case = TRUE) { if (isTRUE(ignore_case)) { - bullets <- c( + cli_abort(c( "Backend does not support case insensitve {.fn str_like}.", i = "Set ignore_case = FALSE for case sensitive match.", i = "Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match." - ) - abort(bullets) + )) } else { sql_expr(!!string %LIKE% !!pattern) } diff --git a/tests/testthat/_snaps/backend-.md b/tests/testthat/_snaps/backend-.md index 358ebd8b1..30225f290 100644 --- a/tests/testthat/_snaps/backend-.md +++ b/tests/testthat/_snaps/backend-.md @@ -45,7 +45,7 @@ test_translate_sql(str_like(x, "abc", ignore_case = TRUE)) Condition Error in `str_like()`: - ! Backend does not support case insensitve {.fn str_like}. + ! Backend does not support case insensitve `str_like()`. i Set ignore_case = FALSE for case sensitive match. i Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match. diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index ada327233..954f945fe 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -34,6 +34,6 @@ The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. Output # Source: table<`x`> [0 x 1] - # Database: sqlite 3.45.0 [:memory:] + # Database: sqlite 3.41.2 [:memory:] # i 1 variable: y From 029aaee3bfb81ca62363d67adca00e3ebbb5fc02 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 4 Apr 2024 15:38:24 +0100 Subject: [PATCH 04/12] updated sqlite version --- tests/testthat/_snaps/tbl-sql.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index 954f945fe..13680eec6 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -32,8 +32,10 @@ Condition Warning: The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. + i The deprecated feature was likely used in the dbplyr package. + Please report the issue at . Output # Source: table<`x`> [0 x 1] - # Database: sqlite 3.41.2 [:memory:] + # Database: sqlite 3.45.2 [:memory:] # i 1 variable: y From 127a93279eb58e3c2b48f848a478b74dce4c81e1 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 4 Apr 2024 15:52:59 +0100 Subject: [PATCH 05/12] another snap --- tests/testthat/_snaps/tbl-sql.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index 13680eec6..0f99e353a 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -32,8 +32,6 @@ Condition Warning: The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. - i The deprecated feature was likely used in the dbplyr package. - Please report the issue at . Output # Source: table<`x`> [0 x 1] # Database: sqlite 3.45.2 [:memory:] From 43f9fb535cb793ca4db638cffe5114873de19f66 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:32:27 +0100 Subject: [PATCH 06/12] updates --- NEWS.md | 1 + R/backend-.R | 3 ++- tests/testthat/test-backend-.R | 7 +++++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0f1748db9..ff8f0c3cc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,5 @@ # dbplyr (development version) +* The translation of `stringr::str_like()` now defaults to use case-sensitive LIKE when argument `ignore_case` is set as FALSE instead of when this argument is set as TRUE.(@edward-burn, #1488) # dbplyr 2.5.0 diff --git a/R/backend-.R b/R/backend-.R index b3c2f845e..c7a576cfc 100644 --- a/R/backend-.R +++ b/R/backend-.R @@ -280,12 +280,13 @@ base_scalar <- sql_translator( str_trim = sql_str_trim, str_c = sql_paste(""), str_sub = sql_str_sub("SUBSTR"), + # https://docs.getdbt.com/sql-reference/like is typically case sensitive str_like = function(string, pattern, ignore_case = TRUE) { if (isTRUE(ignore_case)) { cli_abort(c( "Backend does not support case insensitve {.fn str_like}.", i = "Set ignore_case = FALSE for case sensitive match.", - i = "Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match." + i = "Or use `tolower()` on both arguments to achieve a case insensitive match." )) } else { sql_expr(!!string %LIKE% !!pattern) diff --git a/tests/testthat/test-backend-.R b/tests/testthat/test-backend-.R index 741305e2e..06b86d361 100644 --- a/tests/testthat/test-backend-.R +++ b/tests/testthat/test-backend-.R @@ -101,9 +101,12 @@ test_that("lead and lag translate n to integers", { # strings ----------------------------------------------------------------- -test_that("can translate case insensitive like", { +test_that("can only translate case sensitive str_like", { local_con(simulate_dbi()) - test_translate_sql(str_like(x, "abc", ignore_case = FALSE)) + expect_equal(test_translate_sql(str_like(x, "abc", ignore_case = FALSE)), + sql("`x` LIKE 'abc'")) + expect_equal(test_translate_sql(str_like(x, "ABC", ignore_case = FALSE)), + sql("`x` LIKE 'ABC'")) expect_snapshot( test_translate_sql(str_like(x, "abc", ignore_case = TRUE)), error = TRUE From a8d36a6974082c8b7d8068ff78419b0ceaaafe7a Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Wed, 10 Apr 2024 11:23:57 +0100 Subject: [PATCH 07/12] formatting --- tests/testthat/test-backend-.R | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-backend-.R b/tests/testthat/test-backend-.R index 06b86d361..8f42799ba 100644 --- a/tests/testthat/test-backend-.R +++ b/tests/testthat/test-backend-.R @@ -103,10 +103,14 @@ test_that("lead and lag translate n to integers", { test_that("can only translate case sensitive str_like", { local_con(simulate_dbi()) - expect_equal(test_translate_sql(str_like(x, "abc", ignore_case = FALSE)), - sql("`x` LIKE 'abc'")) - expect_equal(test_translate_sql(str_like(x, "ABC", ignore_case = FALSE)), - sql("`x` LIKE 'ABC'")) + expect_equal( + test_translate_sql(str_like(x, "abc", ignore_case = FALSE)), + sql("`x` LIKE 'abc'") + ) + expect_equal( + test_translate_sql(str_like(x, "ABC", ignore_case = FALSE)), + sql("`x` LIKE 'ABC'") + ) expect_snapshot( test_translate_sql(str_like(x, "abc", ignore_case = TRUE)), error = TRUE From 1cfd161032e0aa65d1619b9638b513c12c75d4f8 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Wed, 10 Apr 2024 21:26:12 +0100 Subject: [PATCH 08/12] change ignore_case default to FALSE --- R/backend-.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/backend-.R b/R/backend-.R index c7a576cfc..2e6111b81 100644 --- a/R/backend-.R +++ b/R/backend-.R @@ -281,7 +281,7 @@ base_scalar <- sql_translator( str_c = sql_paste(""), str_sub = sql_str_sub("SUBSTR"), # https://docs.getdbt.com/sql-reference/like is typically case sensitive - str_like = function(string, pattern, ignore_case = TRUE) { + str_like = function(string, pattern, ignore_case = FALSE) { if (isTRUE(ignore_case)) { cli_abort(c( "Backend does not support case insensitve {.fn str_like}.", From 608631cafa8eb9922dcd644cb96e13ef1f7b3974 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Fri, 3 May 2024 21:28:02 +0100 Subject: [PATCH 09/12] update snaps --- tests/testthat/_snaps/backend-.md | 4 ++-- tests/testthat/_snaps/tbl-sql.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testthat/_snaps/backend-.md b/tests/testthat/_snaps/backend-.md index 30225f290..c4684034a 100644 --- a/tests/testthat/_snaps/backend-.md +++ b/tests/testthat/_snaps/backend-.md @@ -39,7 +39,7 @@ Error in `x$id`: ! $ operator is invalid for atomic vectors -# can translate case insensitive like +# can only translate case sensitive str_like Code test_translate_sql(str_like(x, "abc", ignore_case = TRUE)) @@ -47,7 +47,7 @@ Error in `str_like()`: ! Backend does not support case insensitve `str_like()`. i Set ignore_case = FALSE for case sensitive match. - i Note, using `tolower()` to cast string and pattern to lower case would also achieve a case insenitive match. + i Or use `tolower()` on both arguments to achieve a case insensitive match. # default raw escapes translated correctly diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index e062e912e..91259fd97 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -32,5 +32,5 @@ Condition Warning: The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. - + From 8bd216c6a6b8536bd51b0caffbe5d528b36941dc Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Fri, 3 May 2024 21:34:48 +0100 Subject: [PATCH 10/12] delete extra line --- tests/testthat/_snaps/tbl-sql.md | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index 91259fd97..f207e90ea 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -32,5 +32,4 @@ Condition Warning: The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. - From a8198be525e1228e43d37a37a33b07e0a9bd233b Mon Sep 17 00:00:00 2001 From: simonpcouch Date: Thu, 31 Oct 2024 16:46:04 -0500 Subject: [PATCH 11/12] clarify phrasing, use tidy style --- NEWS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 7bf6d05f7..a70ed7323 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # dbplyr (development version) -* The translation of `stringr::str_like()` now defaults to use case-sensitive LIKE when argument `ignore_case` is set as FALSE instead of when this argument is set as TRUE.(@edward-burn, #1488) + +* Corrected translation of `stringr::str_like()` to use case-sensitive + `LIKE` when argument `ignore_case` is set as `FALSE` (@edward-burn, #1488). * `clock::add_years()` translates to correct SQL on Spark (@ablack3, #1510). From 9ca8c481a653a62daf5e2d479cd3f7a4fe1c1e92 Mon Sep 17 00:00:00 2001 From: simonpcouch Date: Thu, 31 Oct 2024 16:48:02 -0500 Subject: [PATCH 12/12] proofread error message --- R/backend-.R | 8 ++++---- tests/testthat/_snaps/backend-.md | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/R/backend-.R b/R/backend-.R index 2e6111b81..3ce0b50a4 100644 --- a/R/backend-.R +++ b/R/backend-.R @@ -280,13 +280,13 @@ base_scalar <- sql_translator( str_trim = sql_str_trim, str_c = sql_paste(""), str_sub = sql_str_sub("SUBSTR"), - # https://docs.getdbt.com/sql-reference/like is typically case sensitive + # https://docs.getdbt.com/sql-reference/like is typically case sensitive (#1490) str_like = function(string, pattern, ignore_case = FALSE) { if (isTRUE(ignore_case)) { cli_abort(c( - "Backend does not support case insensitve {.fn str_like}.", - i = "Set ignore_case = FALSE for case sensitive match.", - i = "Or use `tolower()` on both arguments to achieve a case insensitive match." + "Backend does not support case insensitive {.fn str_like}.", + i = "Set {.code ignore_case = FALSE} for case sensitive match.", + i = "Use {.fn tolower} on both arguments to achieve a case insensitive match." )) } else { sql_expr(!!string %LIKE% !!pattern) diff --git a/tests/testthat/_snaps/backend-.md b/tests/testthat/_snaps/backend-.md index dadcfcd27..3760aa7ae 100644 --- a/tests/testthat/_snaps/backend-.md +++ b/tests/testthat/_snaps/backend-.md @@ -53,9 +53,9 @@ test_translate_sql(str_like(x, "abc", ignore_case = TRUE)) Condition Error in `str_like()`: - ! Backend does not support case insensitve `str_like()`. - i Set ignore_case = FALSE for case sensitive match. - i Or use `tolower()` on both arguments to achieve a case insensitive match. + ! Backend does not support case insensitive `str_like()`. + i Set `ignore_case = FALSE` for case sensitive match. + i Use `tolower()` on both arguments to achieve a case insensitive match. # default raw escapes translated correctly