diff --git a/NEWS.md b/NEWS.md index e6449d3f..d83e3f2b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # odbc (development version) +* Raises "Cancelling previous query" warnings from R rather than from Rcpp when + a connection has a current result to avoid possible incorrect resource + unwinds with `options(warn = 2)` (#797). + * New `odbc::snowflake()` makes it easier to connect to Snowflake, automatically handling authentication correctly on platforms that provide Snowflake-native OAuth credentials (@atheriel, #662). diff --git a/R/RcppExports.R b/R/RcppExports.R index 7047d58c..ffa33bda 100644 --- a/R/RcppExports.R +++ b/R/RcppExports.R @@ -13,6 +13,10 @@ odbc_connect <- function(connection_string, timezone = "", timezone_out = "", en .Call(`_odbc_odbc_connect`, connection_string, timezone, timezone_out, encoding, bigint, timeout, r_attributes_) } +has_result <- function(p) { + .Call(`_odbc_has_result`, p) +} + connection_info <- function(p) { .Call(`_odbc_connection_info`, p) } diff --git a/R/dbi-connection.R b/R/dbi-connection.R index 131916b3..2b60f8aa 100644 --- a/R/dbi-connection.R +++ b/R/dbi-connection.R @@ -81,6 +81,9 @@ setMethod("dbDisconnect", "OdbcConnection", #' @export setMethod("dbSendQuery", c("OdbcConnection", "character"), function(conn, statement, params = NULL, ..., immediate = FALSE) { + if (has_result(conn@ptr)) { + cli::cli_warn("Cancelling previous query") + } OdbcResult( connection = conn, statement = statement, @@ -109,6 +112,9 @@ setMethod("dbExecute", c("OdbcConnection", "character"), #' @export setMethod("dbSendStatement", c("OdbcConnection", "character"), function(conn, statement, params = NULL, ..., immediate = FALSE) { + if (has_result(conn@ptr)) { + cli::cli_warn("Cancelling previous query") + } OdbcResult( connection = conn, statement = statement, diff --git a/src/RcppExports.cpp b/src/RcppExports.cpp index 907f716b..01ccae6b 100644 --- a/src/RcppExports.cpp +++ b/src/RcppExports.cpp @@ -48,6 +48,17 @@ BEGIN_RCPP return rcpp_result_gen; END_RCPP } +// has_result +bool has_result(connection_ptr const& p); +RcppExport SEXP _odbc_has_result(SEXP pSEXP) { +BEGIN_RCPP + Rcpp::RObject rcpp_result_gen; + Rcpp::RNGScope rcpp_rngScope_gen; + Rcpp::traits::input_parameter< connection_ptr const& >::type p(pSEXP); + rcpp_result_gen = Rcpp::wrap(has_result(p)); + return rcpp_result_gen; +END_RCPP +} // connection_info Rcpp::List connection_info(connection_ptr const& p); RcppExport SEXP _odbc_connection_info(SEXP pSEXP) { @@ -365,6 +376,7 @@ static const R_CallMethodDef CallEntries[] = { {"_odbc_list_drivers_", (DL_FUNC) &_odbc_list_drivers_, 0}, {"_odbc_list_data_sources_", (DL_FUNC) &_odbc_list_data_sources_, 0}, {"_odbc_odbc_connect", (DL_FUNC) &_odbc_odbc_connect, 7}, + {"_odbc_has_result", (DL_FUNC) &_odbc_has_result, 1}, {"_odbc_connection_info", (DL_FUNC) &_odbc_connection_info, 1}, {"_odbc_connection_quote", (DL_FUNC) &_odbc_connection_quote, 1}, {"_odbc_connection_release", (DL_FUNC) &_odbc_connection_release, 1}, diff --git a/src/connection.cpp b/src/connection.cpp index 1c730e6b..0eae2f53 100644 --- a/src/connection.cpp +++ b/src/connection.cpp @@ -73,6 +73,11 @@ std::string get_info_or_empty(connection_ptr const& p, short type) { } } +// [[Rcpp::export]] +bool has_result(connection_ptr const& p) { + return (*p)->has_result(); +} + // [[Rcpp::export]] Rcpp::List connection_info(connection_ptr const& p) { SQLUINTEGER getdata_ext; diff --git a/src/odbc_connection.cpp b/src/odbc_connection.cpp index 48239b8d..5dad3614 100644 --- a/src/odbc_connection.cpp +++ b/src/odbc_connection.cpp @@ -4,24 +4,25 @@ namespace odbc { -void odbc_connection::cancel_current_result(bool quiet) { +void odbc_connection::cancel_current_result() { if (current_result_ == nullptr) { return; } - if (!quiet) { - Rcpp::warning("Cancelling previous query"); - } current_result_->statement()->cancel(); current_result_ = nullptr; } +bool odbc_connection::has_result() const { + return current_result_ != nullptr; +} + void odbc_connection::set_current_result(odbc_result* r) { if (r == current_result_) { return; } - cancel_current_result(r == nullptr); + cancel_current_result(); current_result_ = r; } diff --git a/src/odbc_connection.h b/src/odbc_connection.h index 4598671e..e018614a 100644 --- a/src/odbc_connection.h +++ b/src/odbc_connection.h @@ -38,8 +38,9 @@ class odbc_connection { bool supports_transactions() const; bool get_data_any_order() const; - void cancel_current_result(bool quiet); + void cancel_current_result(); void set_current_result(odbc_result* r); + bool has_result() const; cctz::time_zone timezone() const; std::string timezone_out_str() const; diff --git a/src/odbc_result.cpp b/src/odbc_result.cpp index c5d255b4..3c4270a9 100644 --- a/src/odbc_result.cpp +++ b/src/odbc_result.cpp @@ -16,7 +16,7 @@ odbc_result::odbc_result( bound_(false), output_encoder_(Iconv(c_->encoding(), "UTF-8")) { - c_->cancel_current_result(false); + c_->cancel_current_result(); if (immediate) { s_ = std::make_shared();