feat: dbUnquoteIdentifier() creates Id() objects without component names
#422
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time. See the real-time status of this PR on the Aviator webapp. Use the Aviator Chrome Extension to see the status of your PR within GitHub.
|
#Conflicts: # tests/testthat/test-dbUnquoteIdentifier_DBIConnection.R
|
So, we're losing the names in the output of Could be that we lose (or have already lost) |
|
Hmmmm, lets explicitly test for that, and maybe see if we can repair in |
|
Hmmm, it appears this only affects autocomplete (and I'm not sure in what context) so it might be ok to break temporarily. The cran github org seems to be down right now, but it looks like maybe only RMariaDB is the only So all-in-all I think this is likely to be a low risk change. |
But overall I think this is unlikely to cause problems for folks. |
There was a problem hiding this comment.
Thanks. Do we still need this right now, if the user has a way to create Id objects without naming the components? Why is it important that dbUnquoteIdentifier() returns unnamed Id objects?
How do you feel about making this opt-in, with a new named argument to dbUnquoteIdentifier() ? That would give me a little more peace of mind.
| if (is(x, "SQL")) { | ||
| id_rx <- '(?:"((?:[^"]|"")+)"|([^". ]+))' | ||
| # Determine quoting character | ||
| quote_char <- substr(dbQuoteIdentifier(conn, ""), 1, 1) |
There was a problem hiding this comment.
Should we handle the case of brackets [] used for quoting?
| if (is(x, "SQL") || is.character(x)) { | ||
| x <- lapply(x, unquote, quote_char = quote_char) | ||
| lapply(x, Id) | ||
| } else if (is(x, "Id")) { |
There was a problem hiding this comment.
That would be "UnnamedId" after #429.
krlmlr
changed the title
dbUnquoteIdentifier() creates Id() objects without component names
|
Thanks! |
Fixes #421