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

MSSQL connection. Errors in dplyr select() after arrange #3062

Closed
pssguy opened this issue Aug 29, 2017 · 13 comments
Closed

MSSQL connection. Errors in dplyr select() after arrange #3062

pssguy opened this issue Aug 29, 2017 · 13 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@pssguy
Copy link

pssguy commented Aug 29, 2017

I am attempting to use an MSSQL connection and hitting this issue

I first replicate the example from the dbplyr intro

library(odbc)
library(DBI)

 library(tidyverse) 
 library(dbplyr)

con <- DBI::dbConnect(RSQLite::SQLite(), ":memory:")
DBI::dbWriteTable(con, "iris", iris)
iris2 <- tbl(con, "iris")

iris2 %>% 
  arrange(Species) %>% 
  select(Sepal.Length)

This works fine

Now with an MSSQL connection

con2 <- dbConnect(odbc::odbc(), DSN = "premier")
DBI::dbWriteTable(con2, "iris", iris, overwrite=TRUE)
iris9 <- tbl(con2, "iris")

test1 <-iris9 %>% 
  arrange(Species) %>% 
  select(Sepal.Length)

test1

# Error: <SQL> 'SELECT TOP 1000 "Sepal.Length" AS "Sepal.Length" FROM (SELECT * FROM "iris" ORDER BY "Species") "odeiuzmtqh"' nanodbc/nanodbc.cpp:1587: 42000: [Microsoft][SQL Server Native Client 11.0][SQL Server]The ORDER BY clause is invalid in views, inline functions, derived tables, subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified.

So it appears as though error has a conflict re use of TOP

Reversing the select and arrange commands

test2 <-iris9 %>% 
  select(Sepal.Length) %>% 
  arrange(Species) 

test2 # No error

This works in a simple example but I will sometimes need to arrange data prior to other processes in a pipe

When I look at the problem code it does not exactly replicate error i.e no mention of Top 1000

test1 %>% show_query()
# <SQL>
# SELECT "Sepal.Length" AS "Sepal.Length"
# FROM (SELECT *
# FROM "iris"
# ORDER BY "Species") "omlcgfsrjt"
  

Trying several alternatives in SQL


SELECT "Sepal.Length" AS "Sepal.Length"
 FROM (SELECT *
 FROM "iris"
 ORDER BY "Species") "omlcgfsrjt"

nanodbc/nanodbc.cpp:1587: 42000: [Microsoft][SQL Server Native Client 11.0][SQL Server]The ORDER BY clause is invalid in views, inline functions, derived tables, subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified. 
Failed to execute SQL chunk

something that looks like error code


SELECT TOP 1000 "Sepal.Length" AS "Sepal.Length"
 FROM (SELECT *
 FROM "iris"
 ORDER BY "Species") "omlcgfsrjt"

  nanodbc/nanodbc.cpp:1587: 42000: [Microsoft][SQL Server Native Client 11.0][SQL Server]The ORDER BY clause is invalid in views, inline functions, derived tables, subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified. 
Failed to execute SQL chunk

replacing top 1000 in sub-query produces the desired output


SELECT  "Sepal.Length" AS "Sepal.Length"
 FROM (SELECT TOP 1000 *
 FROM "iris"
 ORDER BY "Species") "omlcgfsrjt"

Not sure if this is an error or just something that has not yet been addressed for MSSQL.

p.s. Why no issues option under dbplyr?

@pssguy pssguy changed the title MSSQL connection. Errors in dplyr:select after arrange MSSQL connection. Errors in dplyr select() after arrange Aug 29, 2017
@edgararuiz-zz
Copy link

Hi @pssguy , it looks like this is a nuance of the queries MSSQL accepts. In order for it to work, the query from test1 that originally translates to this:

# <SQL>
# SELECT "Sepal.Length" AS "Sepal.Length"
# FROM (SELECT *
# FROM "iris"
# ORDER BY "Species") "omlcgfsrjt"

Should actually translate to this:

# <SQL>
# SELECT "Sepal.Length" AS "Sepal.Length"
# FROM "iris"
# ORDER BY "Species"

We will need to figure a way to optimize this query.

@pssguy
Copy link
Author

pssguy commented Sep 7, 2017

@edgararuiz Thanks. Not just optimizing, of course. Currently an error is thrown

@edgararuiz-zz
Copy link

Right, optimize was probably the wrong word choice, I meant we'll need to figure out a way to merge the two SQL query layers to prevent the error from happening.

@imanuelcostigan
Copy link
Contributor

@edgararuiz you may want to look at RSQLServer's sql_select() method which deals with a number of SQL Server's SELECT idiosyncracies.

@edgararuiz-zz
Copy link

Thanks @imanuelcostigan ! I'll take a look

@hadley
Copy link
Member

hadley commented Oct 23, 2017

@edgararuiz do you want to work on a PR for this issue?

@hadley hadley added bug an unexpected problem or unintended behavior database labels Oct 23, 2017
@edgararuiz-zz
Copy link

Yes, I'll be happy to

@hadley
Copy link
Member

hadley commented Nov 2, 2017

Minimal reprex:

library(dplyr, warn.conflicts = FALSE)
library(dbplyr, warn.conflicts = FALSE) 

con <- DBI::dbConnect(RSQLite::SQLite(), ":memory:")
mf <- copy_to(con, data.frame(x = 1:5, y = 5:1), name = "test")

mf %>%
  arrange(x) %>%
  select(y) %>%
  show_query()
#> <SQL>
#> SELECT `y`
#> FROM (SELECT *
#> FROM `test`
#> ORDER BY `x`)

DBI::dbGetQuery(con, "SELECT y FROM test ORDER BY x")
#>   y
#> 1 5
#> 2 4
#> 3 3
#> 4 2
#> 5 1

Ideally this would only generate one query because conceptually the select happens after the arrange.

I think that implies we can fix this issue by reordering select_query_clauses(), which currently looks like this:

  present <- c(
    where =    length(x$where) > 0,
    group_by = length(x$group_by) > 0,
    having =   length(x$having) > 0,
    select =   !identical(x$select, sql("*")),
    distinct = x$distinct,
    order_by = length(x$order_by) > 0,
    limit    = !is.null(x$limit)
  )

Currently select comes before arrange when really it should come afterwards.

And indeed if we move select to the end then we get:

SELECT `y`
FROM `test`
ORDER BY `x`

It remains to consider if this is actually correct - i.e. are there situations when this change would yield invalid SQL

@hadley
Copy link
Member

hadley commented Nov 2, 2017

Ah I think the problem with performing this optimisation is this query:

memdb_frame(x = 1:2) %>%
    arrange(x) %>%
    mutate(x = -x)

This should return c(-1, -2), but if we collapse the query as described above we generate:

SELECT -`x` AS `x`
FROM `gatlqlicge`
ORDER BY `x`

which yields c(-2, -1) because the ORDER BY clause uses aliases defined in SELECT (as described in https://sqlbolt.com/lesson/select_queries_order_of_execution). This means that this optimisation is not possible in general.

But this is a mutate() and the motivation issue is a select(). Can we perform the optimisation at a higher level? I think the answer is no, because select()s can rename variables and this SQL would still be incorrect:

  memdb_frame(x = 1:2, y = 3:2) %>%
    arrange(x) %>%
    select(x = y) %>%
    show_query()
#> SELECT `y` AS `x`
#> FROM `bmvfznmfws`
#> ORDER BY `x`

@hadley
Copy link
Member

hadley commented Nov 2, 2017

But maybe we can just do the optimisation when the select doesn't create any aliases

@hadley
Copy link
Member

hadley commented Nov 2, 2017

I tried that and couldn't get it to work 😢

@hadley hadley removed the wip work in progress label Jun 7, 2018
@ghost ghost deleted a comment from hadley Jun 7, 2018
@ghost
Copy link

ghost commented Jun 7, 2018

This issue was moved by hadley to tidyverse/dbplyr/issues/94.

@ghost ghost closed this as completed Jun 7, 2018
@lock
Copy link

lock bot commented Dec 5, 2018

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Dec 5, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

4 participants