From ba8fef57a1dbcbc996405f0761636d099a4c4941 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 22 Oct 2020 19:21:08 +0200 Subject: [PATCH 1/3] options$error support in eng_sql fixes rstudio/rmarkdown#1208 --- R/engine.R | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/R/engine.R b/R/engine.R index 044e81f314..c66e587d06 100644 --- a/R/engine.R +++ b/R/engine.R @@ -546,18 +546,27 @@ eng_sql = function(options) { query = interpolate_from_env(conn, sql) if (isFALSE(options$eval)) return(engine_output(options, query, '')) - if (is_sql_update_query(query)) { - DBI::dbExecute(conn, query) - data = NULL - } else if (is.null(varname) && max.print > 0) { - # execute query -- when we are printing with an enforced max.print we - # use dbFetch so as to only pull down the required number of records - res = DBI::dbSendQuery(conn, query) - data = DBI::dbFetch(res, n = max.print) - DBI::dbClearResult(res) - } else { - data = DBI::dbGetQuery(conn, query) - } + data = tryCatch({ + if (is_sql_update_query(query)) { + DBI::dbExecute(conn, query) + NULL + } else if (is.null(varname) && max.print > 0) { + # execute query -- when we are printing with an enforced max.print we + # use dbFetch so as to only pull down the required number of records + res = DBI::dbSendQuery(conn, query) + data = DBI::dbFetch(res, n = max.print) + DBI::dbClearResult(res) + data + } else { + DBI::dbGetQuery(conn, query) + } + }, error = function(e) { + if (!options$error) stop(e) + e + }) + + if (inherits(data, "error")) + return(engine_output(options, query, one_string(data))) # create output if needed (we have data and we aren't assigning it to a variable) output = if (length(dim(data)) == 2 && ncol(data) > 0 && is.null(varname)) capture.output({ From c4c9111aefd9b4c390a12a830d027eff4201d578 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 22 Oct 2020 19:31:10 +0200 Subject: [PATCH 2/3] add NEWS bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 902c2565e4..c34e2852a7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # CHANGES IN knitr VERSION 1.31 +- `eng_sql` now correctly captures error if `error = TRUE` in chunk options. (thanks, @colearendt, rstudio/rmarkdown#1208) + - The chunk option `collapse = TRUE` now works as expected when the chunk option `attr.*` or `class.*` is provided. By this change, The chunk option `collapse = TRUE` forces `attr.*` and `class.*` be `NULL` except for the chunk options `attr.source` and `class.source` (thanks, @aosavi @cderv @atusy, #1902 #1906). # CHANGES IN knitr VERSION 1.30 From e029de70525763b820a851ba876e42163d391007 Mon Sep 17 00:00:00 2001 From: Yihui Xie Date: Thu, 22 Oct 2020 12:43:35 -0500 Subject: [PATCH 3/3] Update NEWS.md --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c34e2852a7..2bf24ce42d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # CHANGES IN knitr VERSION 1.31 -- `eng_sql` now correctly captures error if `error = TRUE` in chunk options. (thanks, @colearendt, rstudio/rmarkdown#1208) +- The `sql` engine now correctly captures error with the chunk option `error = TRUE` (thanks, @colearendt, rstudio/rmarkdown#1208). - The chunk option `collapse = TRUE` now works as expected when the chunk option `attr.*` or `class.*` is provided. By this change, The chunk option `collapse = TRUE` forces `attr.*` and `class.*` be `NULL` except for the chunk options `attr.source` and `class.source` (thanks, @aosavi @cderv @atusy, #1902 #1906).