Skip to content

Commit

Permalink
move [R] warnings out of destructor path (#797)
Browse files Browse the repository at this point in the history
Co-authored-by: simonpcouch <[email protected]>
  • Loading branch information
detule and simonpcouch authored May 15, 2024
1 parent 889242b commit 9cdcfb0
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 7 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
4 changes: 4 additions & 0 deletions R/RcppExports.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions R/dbi-connection.R
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions src/RcppExports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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},
Expand Down
5 changes: 5 additions & 0 deletions src/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions src/odbc_connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/odbc_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/odbc_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<nanodbc::statement>();
Expand Down

0 comments on commit 9cdcfb0

Please sign in to comment.