Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move knitr SQL Engine into DBI? #321

Closed
rnorberg opened this issue Sep 29, 2020 · 4 comments
Closed

Move knitr SQL Engine into DBI? #321

rnorberg opened this issue Sep 29, 2020 · 4 comments

Comments

@rnorberg
Copy link
Contributor

I think the three most commonly used R Markdown chunk engines are R, python, and SQL. I noticed that knitr has a rudimentary python engine, but the reticulate package contains its own, much more complex python engine. In this issue in the knitr repo, someone's asking about passing passing parameters in a SQL chunk and @yihui basically says to ask @javierluraschi because he wrote the SQL engine. I think it makes sense to move the SQL engine into a repo that's all about executing SQL, or maybe to create a second, more sophisticated SQL engine for R Markdown in this repo.

Some ideas for an enhanced SQL engine:

  • A way to pass additional arguments such as immediate, to dbGetQuery/dbExecute via chunk options
  • Allow multiple, semicolon separated, SQL statements in a single chunk
  • Better guessing about whether to execute a query with dbExecute or dbGetQuery (see here)
  • When a query is executed with dbExecute(), emit a message() about the number of affected rows (potentially helpful to the author, easily hidden from output with the message = FALSE chunk option)
  • Parse comments out of SQL code using sqlCommentSpec instead of regex for /* ... */ and/or -- ...
  • A vignette all about SQL chunks

What do you think about adding a knitr SQL engine to the DBI package? I'd be willing to take a shot at a first draft.

@krlmlr
Copy link
Member

krlmlr commented Sep 29, 2020

Thanks. I like the idea of a better engine, I don't think it's a good fit for this repository.

@yihui
Copy link

yihui commented Sep 29, 2020

@rnorberg I'll be happy if you could find a new home for the sql engine, no matter where the new home is. If you cannot, it'll be equally great if you find someone who could spend more time on maintaining it in knitr (it could be you if you want). As I said, I don't have much expertise on databases, so I'm really not a good person to maintain this engine. Thanks!

@rnorberg
Copy link
Contributor Author

The sql engine is all about running SQL queries in knitr, so if it doesn't belong in the package for running SQL queries (DBI), I'd say it belongs in knitr. The only other option I can think of is to put it in its own package, but I kind of like that a change in knitr that breaks the sql engine functionality will yield a failed test :)

@yihui, I'd be happy to work on it/maintain it in the knitr repo. I think I'll start with updating the sql engine example in knitr-examples. That will be a good guardrail to make sure changes to it don't break existing functionality. And it'll give you a chance to see some of my code and you can decide if it's safe to let me near knitr :)

@github-actions
Copy link

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants