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

Question on handling of `Date` with `glue_sql()` #98

Closed
DavisVaughan opened this issue Jul 19, 2018 · 3 comments

Comments

@DavisVaughan
Copy link
Member

commented Jul 19, 2018

Hi there! I love glue_sql() for safe interpolation, but have a question regarding how it (and DBI underneath) handle Date objects.

I use SQL Server 2016.

I have a query that drops rows when a refresh is required. Something like this:

library(dbplyr)
library(glue)
library(rlang)

test_con <- simulate_mssql()

first_new_date <- as.Date("2014-01-01")
target_table <- sym("my_table")

sql_drop <- "
DELETE
FROM {target_table}
WHERE {target_table}.end_date >= {first_new_date}
"

# I get this
glue_sql(
  sql_drop, 
  target_table = target_table, 
  first_new_date = first_new_date, 
  .con = test_con
)
#> <SQL> DELETE
#> FROM my_table
#> WHERE my_table.end_date >= 2014-01-01

# I want this
glue_sql(
  sql_drop, 
  target_table = target_table, 
  first_new_date = as.character(first_new_date), 
  .con = test_con
)
#> <SQL> DELETE
#> FROM my_table
#> WHERE my_table.end_date >= '2014-01-01'

Created on 2018-07-19 by the reprex package (v0.2.0).

As you can see, the Date is returned without quotes. Is this ever useful? I don't know a ton about every other flavor of SQL, but I don't know of any that can use unquoted dates.

Regarding DBI, I was going to suggest that you could use dbQuoteLiteral() rather than dbQuoteString() in glue:::sql_quote_transformer(), but it seems to try and print the time zone for Date objects when it coerces them to character. This doesn't work with SQL Server, and I'm not sure where it would apply.

DBI::dbQuoteLiteral(test_con, first_new_date)
#> <SQL> '2014-01-01 UTC'

Is the best solution just to coerce to character first, then run glue_sql()? Or is it worth looking into further? Thanks!

@DavisVaughan

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2018

Oh wow, this totally works as is. I think I had a syntax error in another part of the query. So sorry!

@DavisVaughan

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2018

I take it back, this is actually a more serious problem than I initially thought. On further review, it looks like the query generated using a Date object (returning the unquoted date in the WHERE clause) will run without error in SQL Server, but the WHERE clause is invalid, and SQL Server silently ignores it. This results in all rows of the table being dropped!

Using the single quoted version of the date from first coercing it to a character produces the results I would expect, but this seems generally problematic as people will likely be inclined to interpolate in Date objects directly like I did.

@DavisVaughan DavisVaughan reopened this Jul 19, 2018

@jimhester jimhester added the bug label Aug 14, 2018

@jimhester jimhester closed this in 821f8e8 Mar 11, 2019

@jimhester

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

This should now be fixed.

library(glue)
con <- DBI::ANSI()

first_new_date <- as.Date("2014-01-01")
target_table <- "my_table"

sql_drop <- "
DELETE
FROM {`target_table`}
WHERE {`target_table`}.end_date >= {first_new_date}
"

glue_sql(
  sql_drop, 
  target_table = target_table, 
  first_new_date = first_new_date, 
  .con = con
)
#> <SQL> DELETE
#> FROM "my_table"
#> WHERE "my_table".end_date >= '2014-01-01'

Created on 2019-03-11 by the reprex package (v0.2.1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.