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.

Sign up for GitHub

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

Jump to bottom

Improve default implementation of dbQuoteString() and dbQuoteIdentifier() #77

Closed
5 tasks done
krlmlr opened this issue Feb 6, 2016 · 6 comments
Closed
5 tasks done

Improve default implementation of dbQuoteString() and dbQuoteIdentifier() #77

krlmlr opened this issue Feb 6, 2016 · 6 comments
Labels
action:enhance

Comments

@krlmlr
Copy link
Member

krlmlr commented Feb 6, 2016 •
edited
Loading

  • Single quotes for Strings: http://stackoverflow.com/a/1992331/946850, escape by repetition~~, escape backslash, escape single quote with backslash~~
  • Double quotes for Database stuff, escape by repetition
  • Vectorize dbQuoteString()
  • Add dots in dbQuoteIdentifier()
  • Return "SQL" object
@hadley
Copy link
Member

hadley commented Mar 16, 2016

I'm not sure we should provide too much by default - most database APIs provide explicit quoting/escaping functions which should be used preferentially to doing this in R.

@imanuelcostigan
Copy link

imanuelcostigan commented Dec 26, 2016 •
edited
Loading

@krlmlr dbQuoteIdentifier should not return a vector which the current implementation does (fails DBItest quote_identifier_not_vectorized) which I assume is based on the reference here but which isn't clear from ?DBI::dbQuoteIdentifier()

@krlmlr
Copy link
Member Author

krlmlr commented Dec 28, 2016

@imanuelcostigan: Yes, that's the plan. I'll look into making the documentation clearer. (Could you please fix the reference link?)

@krlmlr
Copy link
Member Author

krlmlr commented Jan 28, 2017

I'm now convinced that dbQuoteIdentifier() should be vectorized, because this seems to be the standard with most existing backends. We'll figure out another way to provide schema information.

@hadley
Copy link
Member

hadley commented Jan 29, 2017

I think something that creates a schema object is probably the way forward: dbQuoteIdentifer(con, schema("x", "y"))

@krlmlr
Copy link
Member Author

krlmlr commented Jan 29, 2017

@hadley: This looks complicated. I'd like to keep the vectorized nature and avoid using new classes if at all possible.

Please see #24 (comment) for the current design. Happy to take your feedback there.

@krlmlr krlmlr closed this as completed in 2e904e7 Jan 31, 2017
krlmlr added a commit that referenced this issue Jan 31, 2017
@krlmlr
Merge branch 'f-prose'. Fixes #77 and #113.
384383e 
- 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).
krlmlr added a commit that referenced this issue Jan 31, 2017
@krlmlr
Merge tag 'v0.5-15'
30fc11e 
- 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).
krlmlr added a commit that referenced this issue Mar 10, 2017
@krlmlr
Merge tag 'v0.6'
76d8d65 
- 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.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
action:enhance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@hadley @krlmlr @imanuelcostigan