Change argument names of sqlInterpolate() and sqlParseVariables() #147

krlmlr · 2016-12-01T14:55:05Z

Use conn and sql instead of _con and _sql.

Also add .dots argument: #140 (comment).

The text was updated successfully, but these errors were encountered:

- Remove `valueClass = "logical"` for those generics where the return value is meaningless, to allow backends to return invisibly (#135). - New `dbQuoteIdentifier(DBIConnection, list)` to support quoting multi-component identifiers such as tables in a schema (#71). DBI backends should implement this method for schema support. - Renamed arguments to `sqlInterpolate()` and `sqlParseVariables()`, the former gains a `.dots` argument (#140, #147). until RSQLite 1.1 has been built for OS X - Remove `max.connections` requirement from documentation (#56). - Enable `dbBind()` example (#136). - Use roxygen2 inheritance to copy DBI specification to this package. - Avoid using braces in the definitions of generics if possible, so that standard generics can be detected (#146). - Change `omegahat.org` URL to `omegahat.net`, the particular document still doesn't exist below the new domain. - Use `tic` package for building documentation.

krlmlr · 2016-12-15T20:44:07Z

@bborgesr @javierluraschi: I think I have found a way to declare the generic so that it's compatible with both the old and new signatures.

bborgesr · 2016-12-15T21:26:12Z

@krlmlr, That's not yet implemented, right? I still get an error...

krlmlr · 2016-12-15T21:38:18Z

No, please watch this issue. (And I'm not so sure anymore... We'll figure out something.)

krlmlr · 2016-12-16T10:45:02Z

Could you please test your packages with

devtools::install_github("rstats-db/DBI@b-%23147-dots-only")

? It's a bit hacky (see diff), but should work with both the old and the new interfaces. If it works, this allows us to:

release DBI 0.6 independently of pool and sparklyr
upgrade pool and sparklyr at your convenience (ideally, before release of DBI 0.7; the GitHub version should start using the new interface as soon as possible)
remove the hack with the DBI 0.7 release

See ?sqlInterpolate and ?sqlParseVariables for details. Thanks.

bborgesr · 2016-12-16T12:36:46Z

Wow, clever! It works for me. @krlmlr, please let me know when you release DBI to CRAN (with this change) and I'll make sure to update the implementations of sqlInterpolate and sqlParseVariables to use the new signature as soon as possible.

krlmlr · 2016-12-16T13:33:35Z

Good news! I'm not releasing to CRAN any time soon, but you should be able to update the implementations to the new signature on GitHub (and use Remotes: rstats-db/DBI in DESCRIPTION) as soon as possible after this issue is closed.

krlmlr · 2016-12-16T13:35:08Z

On the other hand, why force people to use the GitHub version of DBI? The safer approach would be indeed to wait for the CRAN release of DBI.

javierluraschi · 2016-12-16T22:29:38Z

@krlmlr sparklyr works correctly with branch rstats-db/DBI@b-%23147-dots-only. Is this expected? In that case, I'll close my PR to sparklyr!

- `sqlParseVariables()` and `sqlInterpolate()` have `...` as exported formals. DBI drivers are expected to implement `sqlParseVariables(conn, sql, ..., .dots)` and `sqlInterpolate(conn, sql, ...)`. Dispatch occurs manually, overriding default S4 logic. This is a temporary workaround, DBI 0.7 will export the correct signature, and backends not adhering to this signature will not be able to load (#147).

krlmlr · 2016-12-24T10:20:16Z

Will release DBI to CRAN soon, if all revdep checks pass, and then change the signature of these functions in the dev version.

javierluraschi · 2017-01-09T01:38:28Z

@krlmlr with the dev version of DBI in sparklyr I'm hitting this:

library(sparklyr)
library(DBI)

spark_install(version = "2.0.1")
sc <- spark_connect(master = "local", version = "2.0.1")
sqlParseVariables(sc, sql = "SELECT 1")
Error: evaluation nested too deeply: infinite recursion / options(expressions=)?
Error during wrapup: evaluation nested too deeply: infinite recursion / options(expressions=)?

To exit cleanly then run:

spark_disconnect_all()

Is this expected?

krlmlr · 2017-01-30T22:33:06Z

@javierluraschi: No. I guess this means that I revert my evil, evil hack, and we coordinate the release of DBI and sparklyr. What would be a convenient date for you?

BTW, sorry it took me so long. I thought I'd need to install Spark, but you actually did submit a reprex ;-) Neat!

This reverts commit 78d4316, reversing changes made to 238b676.

krlmlr · 2017-01-31T19:35:35Z

@bborgesr: Since pool isn't on CRAN yet, would you mind adding a Remotes: rstats-db/DBI section (and a versioned dependency on DBI) to DESCRIPTION and switch to the new notation?

- Revert `...` hack for `sqlInterpolate()` and `sqlParseVariables()`, simply renamed arguments (#147). - Added specification from DBItest to methods documentation. Affected methods: `dbConnect()`, `dbDisconnect()`, `dbDataType()`, `dbSendQuery()`, `dbFetch()`, `dbClearResult()`, `dbGetQuery()`, `dbSendStatement()`, `dbExecute()`, `dbQuoteIdentifier()`, `dbQuoteString()`, `dbReadTable()`, `dbWriteTable()`, `dbRemoveTable()`, `dbExistsTable()`, and `dbListTables()` (#129). - Added default implementation for `dbReadTable()`. - Removed `dbQuoteIdentifier(DBIConnection, list)` again. - Improved default implementation of `dbQuoteString()` and `dbQuoteIdentifier()` (#77). - Removed `tryCatch()` call in `dbGetQuery()` (#113).

bborgesr · 2017-02-01T02:39:15Z

@krlmlr: I'm not sure what's the lesser evil: the people who will be annoyed if pool is not compatible with the latest DBI devel, or the people who will be annoyed if pool with not compatible with DBI from CRAN. But I'll certainly switch to the new notation and merge that into master as soon as DBI hits CRAN.

On a different note, this wasn't introduced now, but is there a reason that the con and ... arguments are not documented in the help page for sqlParseVariables? (not that it changes anything on my end, just curious):

krlmlr · 2017-02-01T08:03:45Z

Thanks. I'll double-check the documentation, I wonder why R CMD check hasn't noticed this.

For the time being, you could tag/branch the current state of pool and ask users to install from there if they want compatibility with DBI. If install_github() is your preferred method of installation, it's one of:

remotes::install_github("rstudio/pool") # devel DBI
remotes::install_github("rstudio/pool@stable") # CRAN DBI from stable tag/branch

- Interface changes - Deprecated `dbDriver()` and `dbUnloadDriver()` by documentation (#21). - Renamed arguments to `sqlInterpolate()` and `sqlParseVariables()` to be more consistent with the rest of the interface, and added `.dots` argument to `sqlParseVariables`. DBI drivers are now expected to implement `sqlParseVariables(conn, sql, ..., .dots)` and `sqlInterpolate(conn, sql, ...)` (#147). - Interface enhancements - Removed `valueClass = "logical"` for those generics where the return value is meaningless, to allow backends to return invisibly (#135). - Avoiding using braces in the definitions of generics if possible, so that standard generics can be detected (#146). - Added default implementation for `dbReadTable()`. - All standard generics are required to have an ellipsis (with test), for future extensibility. - Improved default implementation of `dbQuoteString()` and `dbQuoteIdentifier()` (#77). - Removed `tryCatch()` call in `dbGetQuery()` (#113). - Documentation improvements - Finalized first draft of DBI specification, now in a vignette. - Most methods now draw documentation from `DBItest`, only those where the behavior is not finally decided don't do this yet yet. - Removed `max.connections` requirement from documentation (#56). - Improved `dbBind()` documentation and example (#136). - Change `omegahat.org` URL to `omegahat.net`, the particular document still doesn't exist below the new domain. - Internal - Use roxygen2 inheritance to copy DBI specification to this package. - Use `tic` package for building documentation. - Use markdown in documentation.

byapparov · 2020-06-20T09:32:28Z

@krlmlr I would like to pass parameters into a sql chunk and was wondering if you could point me to the right direction.

I have asked this here: r-dbi/bigrquery#391 :

I am trying to work out if there is a way to pass parameters to the sql chunks connected to bigquery:

# {r setup}
library(bigrquery)
bq_auth(path = "access_token.json")
db <- dbConnect(
  bigquery(), 
  dataset = 'my_data', 
  project = 'my-project', 
  use_legacy_sql = FALSE
)

parameter_value = 10L

-- {sql, echo=FALSE, connection=db, output.var="x"}
SELECT @parameter_value

# {r output}
print(x)
# I want to see 10 here.

I am happy to contribute if changes are required on bigrquery or DBI packages.

github-actions · 2021-06-21T00:24:38Z

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.

krlmlr added the action:design label Dec 1, 2016

krlmlr mentioned this issue Dec 1, 2016

sqlInterpolate() and sqlParseVariables(): Change of interface sparklyr/sparklyr#351

Closed

krlmlr added this to the 0.6 milestone Dec 1, 2016

krlmlr changed the title ~~Change argument names pf sqlInterpolate() and sqlParseVariables()~~ Change argument names of sqlInterpolate() and sqlParseVariables() Dec 1, 2016

krlmlr self-assigned this Dec 1, 2016

krlmlr added the in progress label Dec 1, 2016

krlmlr mentioned this issue Dec 1, 2016

New sqlInterpolateList() generic #140

Closed

krlmlr closed this as completed in 0ea8879 Dec 2, 2016

ghost removed the in progress label Dec 2, 2016

krlmlr reopened this Dec 15, 2016

krlmlr added the in progress label Dec 15, 2016

javierluraschi mentioned this issue Dec 16, 2016

WIP: fix dbi 0.5.13 breaking change from #351 sparklyr/sparklyr#371

Closed

krlmlr modified the milestones: 0.7, 0.6 Dec 24, 2016

krlmlr added a commit that referenced this issue Jan 31, 2017

Revert "Merge branch 'b-#147-dots-only'."

b07c4d1

This reverts commit 78d4316, reversing changes made to 238b676.

krlmlr closed this as completed in b9e67fb Jan 31, 2017

ghost removed the in progress label Jan 31, 2017

github-actions bot locked and limited conversation to collaborators Jun 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change argument names of sqlInterpolate() and sqlParseVariables() #147

Change argument names of sqlInterpolate() and sqlParseVariables() #147

krlmlr commented Dec 1, 2016 •

edited

Loading

krlmlr commented Dec 15, 2016 •

edited

Loading

bborgesr commented Dec 15, 2016

krlmlr commented Dec 15, 2016

krlmlr commented Dec 16, 2016

bborgesr commented Dec 16, 2016

krlmlr commented Dec 16, 2016

krlmlr commented Dec 16, 2016

javierluraschi commented Dec 16, 2016 •

edited

Loading

krlmlr commented Dec 24, 2016

javierluraschi commented Jan 9, 2017

krlmlr commented Jan 30, 2017

krlmlr commented Jan 31, 2017

bborgesr commented Feb 1, 2017

krlmlr commented Feb 1, 2017

byapparov commented Jun 20, 2020 •

edited

Loading

github-actions bot commented Jun 21, 2021

Change argument names of sqlInterpolate() and sqlParseVariables() #147

Change argument names of sqlInterpolate() and sqlParseVariables() #147

Comments

krlmlr commented Dec 1, 2016 • edited Loading

krlmlr commented Dec 15, 2016 • edited Loading

bborgesr commented Dec 15, 2016

krlmlr commented Dec 15, 2016

krlmlr commented Dec 16, 2016

bborgesr commented Dec 16, 2016

krlmlr commented Dec 16, 2016

krlmlr commented Dec 16, 2016

javierluraschi commented Dec 16, 2016 • edited Loading

krlmlr commented Dec 24, 2016

javierluraschi commented Jan 9, 2017

krlmlr commented Jan 30, 2017

krlmlr commented Jan 31, 2017

bborgesr commented Feb 1, 2017

krlmlr commented Feb 1, 2017

byapparov commented Jun 20, 2020 • edited Loading

github-actions bot commented Jun 21, 2021

krlmlr commented Dec 1, 2016 •

edited

Loading

krlmlr commented Dec 15, 2016 •

edited

Loading

javierluraschi commented Dec 16, 2016 •

edited

Loading

byapparov commented Jun 20, 2020 •

edited

Loading