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

MS SQL mutate(ifelse()) translation not working #93

Closed
bogdanrau opened this issue Jun 1, 2018 · 8 comments · Fixed by #103
Closed

MS SQL mutate(ifelse()) translation not working #93

bogdanrau opened this issue Jun 1, 2018 · 8 comments · Fixed by #103
Assignees
Labels
bug an unexpected problem or unintended behavior func trans 🌍 Translation of individual functions to SQL wip work in progress

Comments

@bogdanrau
Copy link

bogdanrau commented Jun 1, 2018

I'm running into an issue when using mutate and ifelse (or if) on a SQL Server backend. The following sample query:

df <- tbl(con, in_schema("schema", "table_name")) %>%
   mutate(field = ifelse(field %in% c('test1', 'test2', 'test3'), 'string1', 'string2') %>%
   collect()

... will give the following translation:

SELECT CASE WHEN (("field" IN ('test1', 'test2', 'test3')) = 'TRUE') THEN ('string1') 
            WHEN (("field" IN ('test1', 'test2', 'test3')) =  'FALSE') THEN ('string2') 
            END AS "field"

Which will return an error. The correct translation should be:

SELECT CASE WHEN ("field" IN ('test1', 'test2', 'test3')) THEN 'string1' 
            WHEN ("field" NOT IN ('test1', 'test2', 'test3')) THEN 'string2' 
            END AS "field"

Will correcting this in the mssql_if function create any issues in other queries, or is there a better way to do mutate(ifelse()) in mssql?

mssql_sql_if <- function(cond, if_true, if_false = NULL) {

@bogdanrau
Copy link
Author

bogdanrau commented Jun 1, 2018

Sorry guys. Just opened up a ticket in dplyr instead (as suggested in the readme). Closing this guy. Opened tidyverse/dplyr#3632

@hadley hadley reopened this Jun 7, 2018
@hadley
Copy link
Member

hadley commented Jun 7, 2018

No you should file here; I need to update the readme.

@bogdanrau
Copy link
Author

Thanks Hadley. @edgararuiz, any insight on what a fix might be?

@edgararuiz-zz
Copy link
Contributor

Ok, I think that the update may have to be done in the negating function here:

mssql_not_sql_prefix <- function() {

I'll work on a PR for this

@edgararuiz-zz edgararuiz-zz self-assigned this Jun 18, 2018
@edgararuiz-zz
Copy link
Contributor

Quick update, it looks like your suggestion fixes this issue, but it negatively impacts the custom MS SQL infix function we're using, so I have to work some more on this

mssql_logical_infix <- function(f) {

@bogdanrau
Copy link
Author

Thanks for your help on this @edgararuiz! We're transitioning from a MySQL db to MSSQL and it's been super smooth thus far minus this kink.

@ablack3
Copy link

ablack3 commented Oct 5, 2018

Did this issue get worked out or is there a workaround? I'm having the same problem.

@edgararuiz-zz
Copy link
Contributor

@ablack3 - Yes, I have a PR that fixes it. The PR is ready to be reviewed: #103

@hadley hadley added bug an unexpected problem or unintended behavior wip work in progress func trans 🌍 Translation of individual functions to SQL labels Jan 2, 2019
hadley pushed a commit that referenced this issue Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior func trans 🌍 Translation of individual functions to SQL wip work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants