From 951f63f7249d48bc65a00d85d6d493dc7cdb3cab Mon Sep 17 00:00:00 2001 From: Mikko Marttila <13412395+mikmart@users.noreply.github.com> Date: Wed, 16 Oct 2024 16:46:48 +0100 Subject: [PATCH 1/4] Suggest decor (required for tests) --- DESCRIPTION | 1 + 1 file changed, 1 insertion(+) diff --git a/DESCRIPTION b/DESCRIPTION index 31077fc8a..734d1475c 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -44,6 +44,7 @@ Suggests: callr, cli, DBItest (>= 1.8.0), + decor, gert, gh, hms, From 57af5d994d183cd2604fbf7ff4ed71bcf6fd5035 Mon Sep 17 00:00:00 2001 From: Mikko Marttila <13412395+mikmart@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:35:02 +0100 Subject: [PATCH 2/4] Reword dbFetch() with SELECT warning --- src/SqliteResultImpl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SqliteResultImpl.cpp b/src/SqliteResultImpl.cpp index c3d6cd0ce..79f3d3b52 100644 --- a/src/SqliteResultImpl.cpp +++ b/src/SqliteResultImpl.cpp @@ -312,7 +312,7 @@ cpp11::list SqliteResultImpl::fetch_rows(const int n_max, int& n) { SqliteDataFrame data(stmt, cache.names_, n_max, types_, with_alt_types_); if (complete_ && data.get_ncols() == 0) { - Rf_warning("SQL statements must be issued with dbExecute() or dbSendStatement() instead of dbGetQuery() or dbSendQuery()."); + Rf_warning("`dbGetQuery()`, `dbSendQuery()` and `dbFetch()` should only be used with `SELECT` queries. Did you mean `dbExecute()`, `dbSendStatement()` or `dbGetRowsAffected()`?"); } while (!complete_) { From 082eae309570e61ef92ef3e475bfcaa1702201da Mon Sep 17 00:00:00 2001 From: Mikko Marttila <13412395+mikmart@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:46:12 +0100 Subject: [PATCH 3/4] Update tests --- tests/testthat/test-dbSendQuery.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-dbSendQuery.R b/tests/testthat/test-dbSendQuery.R index ac469fdd4..3eda212e0 100644 --- a/tests/testthat/test-dbSendQuery.R +++ b/tests/testthat/test-dbSendQuery.R @@ -45,7 +45,7 @@ test_that("simple position binding works", { ), "deprecated" ), - "SQL statements" + "`SELECT` queries" ) expect_equal(dbReadTable(con, "t1")$x, c(1, 2)) @@ -64,7 +64,7 @@ test_that("simple named binding works", { bind.data = data.frame(y = 1, x = 2) ) %>% expect_warning("deprecated") %>% - expect_warning("SQL statements") + expect_warning("`SELECT` queries") expect_equal(dbReadTable(con, "t1")$x, c(1, 2)) }) From 311a7c7e8d1af82d46efa32d2f5ddd5b72aed165 Mon Sep 17 00:00:00 2001 From: Mikko Marttila <13412395+mikmart@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:51:13 +0100 Subject: [PATCH 4/4] Add test for original issue --- tests/testthat/test-dbSendQuery.R | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/testthat/test-dbSendQuery.R b/tests/testthat/test-dbSendQuery.R index 3eda212e0..66dd643b4 100644 --- a/tests/testthat/test-dbSendQuery.R +++ b/tests/testthat/test-dbSendQuery.R @@ -173,3 +173,15 @@ test_that("mark UTF-8 encoding on non-ASCII colnames", { expect_equal(Encoding(got), "UTF-8") expect_equal(got, cn_field) }) + +test_that("dbFetch with statement other than SELECT warns reasonably (#523)", { + memoise::forget(warning_once) + con <- dbConnect(SQLite(), ":memory:") + on.exit(dbDisconnect(con), add = TRUE) + + dbWriteTable(con, "t1", data.frame(x = 1, y = 2)) + res <- dbSendStatement(con, "INSERT INTO t1 VALUES (2, 1)") + on.exit(dbClearResult(res), add = TRUE, after = FALSE) + + expect_warning(dbFetch(res), "dbGetRowsAffected") +})