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

Add expand Option to SQL Engine #1357

Merged
merged 1 commit into from
Feb 16, 2017
Merged

Add expand Option to SQL Engine #1357

merged 1 commit into from
Feb 16, 2017

Conversation

saurfang
Copy link
Contributor

@saurfang saurfang commented Feb 8, 2017

SQL chunk is an awesome way to showcase SQL queries in syntax highlighted fashion. The support of interpolation is also great to allow further flexibility. While most the time I like showing uninterpolated SQL which is simple and easy to reason, sometimes I wish I can show interpolated SQL instead. This allows me to construct more complex SQL programmatically but show readers the end results.

This PR adds a new option expand for SQL engine, which would show interpolated SQL query in the code chunk.

I am open to any suggestion for a better name. Let me know if you can think of a better idea. Thank you!

Putting a space between ` so code block works correctly.

` ``{r}
x <- 1
` ``

` ``{sql, connection=db}
SELECT ?x
` ``

` ``{sql, connection=db, expand=TRUE}
SELECT ?x
` ``

image

R/engine.R Outdated
@@ -538,8 +538,11 @@ eng_sql = function(options) {
# assign varname if requested
if (!is.null(varname)) assign(varname, data, envir = knit_global())

# reset query to pre-interpolated if not expanding
if (!isTRUE(options$expand)) query <- options$code
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change the default behavior, i.e. previously the default is to expand the query, and now the default is to show raw query (because options$expand is NULL by default). I'd use

if (isFALSE(options$sql.expand))

so that users have to set sql.expand = FALSE explicitly to present the query that is not interpolated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong but I think the previous default behavior is to show query uninterpolated because it is returning engine_output(options, options$code, output) and options$code is before interpolation.

I am indifferent what should be the default behavior. I have found both equally common in my writing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you are correct. I was wrong. Sorry.

@saurfang
Copy link
Contributor Author

I renamed the option to sql.show_interpolated for clarity because there is already knit_expand concept and I fear it can cause confusion. In additional I added support to show interpolated SQL even when eval=FALSE. This is particularly useful for chunk references as in:

` ``{sql x, connection=db}
SELECT ?x
` ``

` ``{sql x, connection=db, eval=FALSE, sql.show_interpolated=TRUE}
` ``

So I can show interpolated SQL at the appendix section of my writing.

@yihui
Copy link
Owner

yihui commented Feb 16, 2017

Okay, this makes perfect sense to me now. Please add a NEWS item to NEWS.md, your name to DESCRIPTION as a contributor (ctb), and I'll merge it. Thanks!

@yihui yihui added this to the v1.16 milestone Feb 16, 2017
@saurfang saurfang force-pushed the sql_expand branch 2 times, most recently from b17f3b5 to ac9e774 Compare February 16, 2017 05:50
`sql.show_interpolated` chunk option, when set to `TRUE`, will render interpolated SQL query instead. This also works when `eval=FALSE`.

Details:

SQL chunk is awesome way to showcase SQL queries in syntax highlighted fashion. The support of interpolation is also great to allow further flexibility. While most the time I like showing uninterpolated SQL which is simple and easy to reason, sometimes I wish I can show interpolated SQL instead. This allows me to construct more complex SQL programatically but show readers the end results.
@saurfang
Copy link
Contributor Author

Thanks! Done and commits squashed.

@yihui yihui merged commit 8be578e into yihui:master Feb 16, 2017
@yihui
Copy link
Owner

yihui commented Feb 16, 2017

👍

@saurfang saurfang deleted the sql_expand branch February 16, 2017 06:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants