From 839d37afdbd415f9825a15ac403b57a72d3421c8 Mon Sep 17 00:00:00 2001 From: Robert Norberg Date: Mon, 21 Sep 2020 21:20:19 -0400 Subject: [PATCH 1/4] adds additional SQL keywords --- R/engine.R | 31 ++++++++++++++++++++++++++++++- tests/testit/test-sql.R | 18 ++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/R/engine.R b/R/engine.R index 044e81f314..faef7fa439 100644 --- a/R/engine.R +++ b/R/engine.R @@ -492,7 +492,36 @@ is_sql_update_query = function(query) { query = gsub('^\\s*--.*\n', '', query) # remove multi-line comments if (grepl('^\\s*\\/\\*.*', query)) query = gsub('.*\\*\\/', '', query) - grepl('^\\s*(INSERT|UPDATE|DELETE|CREATE|DROP).*', query, ignore.case = TRUE) + sql_update_keywords <- c( + # DDL + "CREATE", + "ALTER", + "DROP", + "GRANT", + "DENY", + "REVOKE", + "ANALYZE", + "AUDIT", + "COMMENT", + "RENAME", + "TRUNCATE", + # DML + "INSERT", + "UPDATE", + "DELETE", + "MERGE", + "CALL", + "EXPLAIN PLAN", + "LOCK", + "UNLOCK" + ) + pattern <- paste0( + # comes out looking like "^\\s*(CREATE|ALTER|...).*" + "^\\s*(", + paste(sql_update_keywords, collapse = "|"), + ".*" + ) + grepl(pattern, query, ignore.case = TRUE) } # sql engine diff --git a/tests/testit/test-sql.R b/tests/testit/test-sql.R index b65c815fb6..2e938b10e8 100644 --- a/tests/testit/test-sql.R +++ b/tests/testit/test-sql.R @@ -32,3 +32,21 @@ assert(is_sql_update_query(c( assert(is_sql_update_query('update foo set a=1')) assert(is_sql_update_query('delete from foo')) assert(is_sql_update_query('insert into foo values(1)')) +assert(is_sql_update_query('alter table foo add column x (int);')) +assert(is_sql_update_query('merge into table foo from (select x, y from bar) as bar2 on foo.x = barr.x when matched then update set foo.y = bar.y;')) +assert(is_sql_update_query('grant select on foo to user1')) +assert(is_sql_update_query('deny select on foo to user1')) +assert(is_sql_update_query('revoke select on foo to user1')) +assert(is_sql_update_query('analyze table foo.bar compute statistics for all columns')) +assert(is_sql_update_query('audit select on hr.employees whenever successful')) +assert(is_sql_update_query("comment on column foo.bar is 'hello world'")) +assert(is_sql_update_query('rename table foo to bar;')) +assert(is_sql_update_query('truncate table foo')) +assert(is_sql_update_query("call sysproc.admin_cmd('REORG TABLE foo')")) +assert(is_sql_update_query('explain plan for select * from foo where bar > 0')) +assert(is_sql_update_query('lock table foo in exclusive mode nowait;')) +assert(is_sql_update_query('unlock tables')) + + + + From 6a993b5eba751a077f0f511c2b86a53cb3916676 Mon Sep 17 00:00:00 2001 From: Robert Norberg Date: Mon, 21 Sep 2020 21:37:49 -0400 Subject: [PATCH 2/4] allow users to force execution of sql chunk s with DBI::dbExecute via sql.is_update_query chunk option --- R/engine.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/engine.R b/R/engine.R index faef7fa439..1c3fd5c517 100644 --- a/R/engine.R +++ b/R/engine.R @@ -575,7 +575,7 @@ 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)) { + if (options$sql.is_update_query || is_sql_update_query(query)) { DBI::dbExecute(conn, query) data = NULL } else if (is.null(varname) && max.print > 0) { From f3f5812df8bbdcaf9c096c1c7ec23b5d3c2b2b2f Mon Sep 17 00:00:00 2001 From: Robert Norberg Date: Wed, 23 Sep 2020 17:53:45 -0400 Subject: [PATCH 3/4] oops! --- R/engine.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/engine.R b/R/engine.R index 1c3fd5c517..e9bbe4263d 100644 --- a/R/engine.R +++ b/R/engine.R @@ -519,7 +519,7 @@ is_sql_update_query = function(query) { # comes out looking like "^\\s*(CREATE|ALTER|...).*" "^\\s*(", paste(sql_update_keywords, collapse = "|"), - ".*" + ").*" ) grepl(pattern, query, ignore.case = TRUE) } From a24b4e887b82b40db0d2b0a80d58cc361411f9f6 Mon Sep 17 00:00:00 2001 From: Robert Norberg Date: Wed, 23 Sep 2020 18:31:41 -0400 Subject: [PATCH 4/4] doesn't fail if sql.is_update_query option isn't specified --- R/engine.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/engine.R b/R/engine.R index e9bbe4263d..c9da2f3a42 100644 --- a/R/engine.R +++ b/R/engine.R @@ -575,7 +575,10 @@ eng_sql = function(options) { query = interpolate_from_env(conn, sql) if (isFALSE(options$eval)) return(engine_output(options, query, '')) - if (options$sql.is_update_query || is_sql_update_query(query)) { + is_update <- options$sql.is_update_query + if (is.null(is_update)) is_update <- is_sql_update_query(query) + if (!is.logical(is_update)) stop2("The 'sql.is_update_query' chunk option must be TRUE or FALSE") + if (is_update) { DBI::dbExecute(conn, query) data = NULL } else if (is.null(varname) && max.print > 0) {