Use parameterised query option in SQL chunk

[byapparov]

closes #1867

[CLAassistant]

All committers have signed the CLA.

[byapparov]

@cderv I have tried this on my version of bigrquery package that supports params and it works as expected.

It seems like quite a small change, but number of if statements that sql engine has is slightly dangerous, unless covered by unit tests. I don't think eng_sql() function has any tests right now.

I would also recommend to deprecate DBI interpolation of parameters gradually for databases that support parameterised queries.

[yihui]

Thanks!

but number of if statements that sql engine has is slightly dangerous

That's true :)

I don't think eng_sql() function has any tests right now.

Currently this is our only test: https://github.com/yihui/knitr-examples/blob/master/115-engine-sql.Rmd

I would also recommend to deprecate DBI interpolation of parameters gradually for databases that support parameterised queries.

That sounds like a good plan to me (but again, I don't think I have enough expertise to make the decision).

[byapparov]

I have checked sqlite documentation and it supports @, : and $ named parameters:

e.g.:

An "at" sign works exactly like a colon, except that the name of the parameter created is @AAAA.

I will later update examples file with the params options, so we have some test coverage for that.

Not sure how these two repositories sync, possibly I should wait till this is merged?

[cderv]

Not sure how these two repositories sync, possibly I should wait till this is merged?

Yeah, in fact they are not really sync between PR but are on master. I need to improve that and implement an integration workflow for such cases.

But you can still open a PR in knitr-example if you want to share / save your example file.

I'll have a look soon at this. You example file could help me for the review. Thanks.
I'll think also about the deprecation you mentioned

[byapparov]

@cderv I have added example to knitr-examples.

I am not sure how you generate .md files from .Rmd, so you might want to generate it your self.

It would be also helpful to update README with the method used to generate md for future contributors.

[yihui]

I think this PR can be merged after you add an item to NEWS.md (unless @cderv has any other feedback). Also feel free to add your name to DESCRIPTION as ctb. Thanks!

It would be also helpful to update README with the method used to generate md for future contributors.

There is an ugly shell script k in the repo, and the examples are knitted with this shell script, e.g., ./k 115-engine-sql.Rmd. Before documenting it in README, we'll need to make it less ugly :)

[cderv]

That is ok with me.

We should also update the documentation for the SQL engine in different places now so that this is documented.

Before documenting it in README, we'll need to make it less ugly :)

Yes this workflow of adding test in knitr-example should be improved - and documented / automated at the right level. Currently it is not easy to setup - I had a hard time doing it if I remember correctly :)
We'll improve that.

[byapparov]

@yihui I have added a line to NEWS and added myself to DESCRIPTION.

Thank you for helping with this, looking forward to using this in my projects )