Improved SQL dbExecute vs dbGetQuery

[rnorberg]

Adds additional SQL "update" keywords (ALTER, GRANT, MERGE, etc) to is_sql_update_query (which is a package internal function and not exported) and allow users to force a query contained in a SQL chunk to be dispatched to DBI::dbExecute or DBI::dbGetQuery via the new sql.is_update_query chunk option.

[CLAassistant]

All committers have signed the CLA.

[dpprdan]

I've also run into this issue.

Details

I.e. the DB driver emits a warning, when you have a SQL statement without one of the current keywords inside a sql chunk. E.g. RPostgres emits

In result_fetch(res@ptr, n = n) :
  Don't need to call dbFetch() for statements, only for queries

Other driver may emit warnings with different wordings, but they are essentially the same, e.g. RSQLite.

I therefore support this PR. However, I would suggest two changes

1. The chunk option should be called something more "formal" e.g. sql.is_statement (is_update_query is a contradiction IIUC, since a statement with UPDATE (usually) does not return a result set, so it isn't a query).
2. For PostgreSQL support I'd add SET, REFRESH and VACUUM as keywords. (But this isn't strictly necessary with the sql.is_* option).

NB: It's possible to have SELECT statements that are not queries (e.g. SELECT INTO), as well as statements with a keyword that are queries (e.g. UPDATE [...] RETURNING), which makes the sql.is_* option the most important element of this PR.

@cderv do you have time to look into this?

[cderv]

@dpprdan thanks for pinging on this issue. I believe this was on long hold due to r-dbi/DBI#321 (comment) - It would be better for the long term that the SQL engine for knitr leaves elsewhere than inside knitr. However, it seems the DBI is not a good place. Engine can be in their own package and SQL feature is not core of knitr directly.

I think this could make sense to have an improved and new engine in its own package now, and that it is maintained with the help of SQL users and expert. What do you think ? I'll run this project idea internally too, I think we can start that in 2023.

[dpprdan]

@cderv That sounds very reasonable. I can see myself in "with the help of SQL users", so feel free to ping me if you think I can assist somewhere.

You probably thought of this already, but it may make sense to consider the glue_sql engine here as well.

[yihui]

Since we have gotten another vote from users, I can definitely consider merging this PR (since it doesn't appear to be too complicated). In the long run, I still think it will be great if we have one or two helpers who could have some soft commitment to maintain this engine in knitr (it may not require a lot of effort, and Posit/RStudio can also send gifts as incentives if necessary).

1. The chunk option should be called something more "formal" e.g. sql.is_statement

I think that's a great suggestion.