Clarify `fetch_no_return_value` test: shouldn't call `dbFetch()` on a `dbSendStatement()` result

[MichaelChirico]

DBItest/R/spec-result-fetch.R

Lines 87 to 97 in f19fb4d

	fetch_no_return_value = function(con, table_name) {
	#'
	#' Calling `dbFetch()` on a result set from a data manipulation query
	#' created by [dbSendStatement()] can
	#' be fetched and return an empty data frame, with a warning.
	query <- paste0("CREATE TABLE ", table_name, " (a integer)")
	res <- local_result(dbSendStatement(con, query))
	expect_warning(rows <- check_df(dbFetch(res)))
	expect_identical(rows, data.frame())
	},

I'm not sure I understand this, and the example is using dbSendStatement() in a way that differs slightly from the documentation I see (which covers INSERT/UPDATE/DELETE but not CREATE statements).

Do you have an example of a backend that implements this as intended? It's also a bit hard to search for.

[krlmlr]

Thanks. When I search for it, I see that the RSQLite, RMariaDB and RPostgres backends should implement this because the check is not excluded there: https://github.com/search?q=org%3Ar-dbi%20fetch_no_return_value&type=code .

Users shouldn't fetch a statement result (side effects), only a query result (data frame output). How can we clarify this in the text and the code?

[MichaelChirico]

Let me see if I understand with a fresh reading of ?dbFetch and ?dbSendStatement. Basically we want dbFetch() to warn on the result of any call to dbSendStament() because that function is used for side-effect queries and not expected to return results. It's really only meant for use with dbSendQuery().

SendStatement() vs. SendQuery() is a subtle enough distinction, I think it helps to emphasize it where it occurs, my eyes tend to focus on dbSend and glaze over the tail part (esp. since I rarely use Statement queries myself).

Some suggestion...

 fetch_no_return_value = function(con, table_name) { 
   #' 
-   #' Calling `dbFetch()` on a result set from a data manipulation query 
+   #' Calling `dbFetch()` on result sets from data manipulation queries
-   #' created by [dbSendStatement()] can 
-   #' be fetched and return an empty data frame, with a warning. 
+   #' created by [dbSendStatement()] returns an empty data frame, with warning.
   query <- paste0("CREATE TABLE ", table_name, " (a integer)") 
  
   res <- local_result(dbSendStatement(con, query)) 
   expect_warning(rows <- check_df(dbFetch(res))) 
   expect_identical(rows, data.frame()) 
 }, 

And then also be sure to mention CREATE TABLE in ?dbSendStatement as one of the statement types, i.e. pull it out of ... here:

e.g., UPDATE, DELETE, INSERT INTO, DROP TABLE, ...

maybe just CREATE/DROP TABLE is enough.