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 Incorrect Translation of between() when not used in filter context #1241

Closed
kmishra9 opened this issue Apr 11, 2023 · 4 comments · Fixed by #1251
Closed

MSSQL Incorrect Translation of between() when not used in filter context #1241

kmishra9 opened this issue Apr 11, 2023 · 4 comments · Fixed by #1251
Labels

Comments

@kmishra9
Copy link

Hey there, stumbling into some more issues related to SQL translations w/ booleans in MSSQL. In this case, the translation for dplyr::between() is the same in both a mutate/summarize context (which doesn't work) and in a filter context (which does).

suppressPackageStartupMessages(library(dplyr))
suppressPackageStartupMessages(library(dbplyr))
suppressPackageStartupMessages(library(lubridate))

lf <-
    lazy_frame(tibble(
        a = 1:10,
        b = 2
    ),
    con = simulate_mssql())

# Succeeds (stock translation)
lf %>% filter(between(x = a, left = b, right = 5))
#> <SQL>
#> SELECT *
#> FROM `df`
#> WHERE (`a` BETWEEN `b` AND 5.0)

# Fails (stock translation)
lf %>% mutate(c = between(x = a, left = b, right = 5))
#> <SQL>
#> SELECT *, `a` BETWEEN `b` AND 5.0 AS `c`
#> FROM `df`

# Succeeds (possible new translation)
lf %>% mutate(c = case_when(between(
    x = a, left = b, right = 3
) ~ TRUE, .default = FALSE))
#> <SQL>
#> SELECT *, CASE WHEN (`a` BETWEEN `b` AND 3.0) THEN 1 ELSE 0 END AS `c`
#> FROM `df`

# Succeeds (possible new translation)
lf %>% mutate(c = if_else(condition = between(
    x = a, left = b, right = 3
), true = TRUE, false = FALSE))
#> <SQL>
#> SELECT *, IIF(`a` BETWEEN `b` AND 3.0, 1, 0) AS `c`
#> FROM `df`

Created on 2023-04-10 with reprex v2.0.2

@mgirlich
Copy link
Collaborator

@kmishra9 I created a PR to fix this issue. You can install it via devtools::install_github("tidyverse/dbplyr"). Can you please check that this fixes the issue.

@kmishra9
Copy link
Author

Hey there, thanks for your work on this.

I installed the updated GitHub version, but in two separate environments (one in my normal development project/environment and then when that didn't work, tried outside of a project installing things from scratch and running the reprex) but was unable to get an updated translation.

In particular, the reprex from above is still translating the same way

I'm fairly sure I had the correct dbplyr version installed and loaded correctly:
image

Let me know if there's any follow-up testing you'd like from me.

@mgirlich
Copy link
Collaborator

Sorry, I forgot to add the PR to the install code. Can you try again after installing via devtools::install_github("tidyverse/dbplyr#1251")?

@mgirlich mgirlich added the MSSQL label Apr 26, 2023
@kmishra9
Copy link
Author

Ok, so after examining how the query is being generated, it's closer but I think it's missing the true and false values to return on an IIF (1 and 0).

Here's the error I get:

nanodbc/nanodbc.cpp:1752: 00000: [FreeTDS][SQL Server]Statement(s) could not be prepared.  [FreeTDS][SQL Server]Incorrect syntax near 'q01'.  [FreeTDS][SQL Server]Incorrect syntax near ')'. 
<SQL> 'SELECT TOP 11 *, IIF(1.0 BETWEEN 0.0 AND 5.0) AS "c"
FROM (
  SELECT *, "member_id" AS "linkage_id"
  FROM "#dbplyr_001"
) "q01"'

Here's the translation in actual MSSQL (same as on simulated):

<SQL>
SELECT *, IIF(1.0 BETWEEN 0.0 AND 5.0) AS "c"
FROM (
  SELECT *, "member_id" AS "linkage_id"
  FROM "#dbplyr_001"
) "q01"

Here's what it needs to be:

<SQL>
SELECT *, IIF(1.0 BETWEEN 0.0 AND 5.0, 1, 0) AS "c"
FROM (
  SELECT *, "member_id" AS "linkage_id"
  FROM "#dbplyr_001"
) "q01"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants