-
Notifications
You must be signed in to change notification settings - Fork 175
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
Query worked in 2.3.4 but not in 2.4.0 #1388
Comments
I'm seeing similar behaviour after upgrading from 2.3.4 to 2.4.0 connecting to an Impala table. When running 2.4.0:
Output
Using |
Adding to report this that this also no longer works: library("dbplyr")
cn <- DBI::dbConnect(odbc::odbc(), .connection_string = "Driver={SQL Server};server=servername;trusted_connection=yes;")
dplyr::tbl(cn, DBI::Id(database = "database", schema = "dbo", table = "table_name"))
This does work for both 2.3.4 and 2.4.0 (changing 'database' to 'catalog'): library("dbplyr")
cn <- DBI::dbConnect(odbc::odbc(), .connection_string = "Driver={SQL Server};server=servername;trusted_connection=yes;")
dplyr::tbl(cn, DBI::Id(catalog= "database", schema = "dbo", table = "table_name")) |
In my case in_schema works correctly for me, I think the problem is that the input of the variable schema and table are in SQL format and not character, as you can see the result is different
|
@whipson We added the check for a |
@carlinstarrs It definitely makes sense that |
@mgirlich yes, I tried using
|
@whipson Can you add the trace of the error. You can get it via |
|
Thanks! This should be fixed in the implyr package. Please open an issue there. |
@mgirlich you said:
But the docs say
|
I'm also starting to wonder if we've massively over indexed on a problem that few people actually have — how common is that people actually want to refer to a table containing a special character? Maybe we should just let |
@hadley I agree. From my point of view, both |
Appears there is no way to call a UDF in SQL now: Before:
Now says whole object doesn't exist |
At a minimum, I think we should make |
Also the requirement to name all the arguments to |
Is |
The big idea in this PR is to simplify the implementation by immediately escaping as soon as we get a table name. That allows user facing functions to accept a wide range of table specifications while internal functions only need to deal with a new `table_path` class. Additionally, only the `table_path` class is vectorised, so we can ensure that user supplies a single table name (or SQL, where appropriate), while still providing vectorised tools internally. Since we escape table ids early we also need a new tool to extract the name of just the table; this is sort of the opposite problem to `table_ident` which needed tools to stitch components together instead of pulling them apart. I've also simplified the implementation a little by taking `I()` to mean "don't escape". My expectation is that `I("foo.bar")`/`I("foo.bar.baz"))` will become the preferred way to specify nested tables. Fixes #1388. Fixes #1396. Fixes #1413. Fixes #1408. Closes #1385. Fixes #1416. Fixes #1404.
This used to work:
But now gets this error message:
The update to 2.0 also broke our queries that included the database name within in_schema (see #552), and we've been using this workaround since then, but now the workaround is broken. Any chance this is something that would get fixed? The issue is the extra quotes around the . after dbo, i.e. the query should be:
Reverting to 2.3.4 fixes it.
The text was updated successfully, but these errors were encountered: