-
Notifications
You must be signed in to change notification settings - Fork 174
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
Regression: migrating from build_sql() to glue_sql2() lost escaping #1351
Comments
A simple Postgres translation also suffers from this error, when previously it was acceptable, suggesting a broader issue:
|
Ah, yea, the straight glue here removes the escaping: 83045c5#diff-1d16344a03920504ed530dc7fa4a10614ffc383ba2c5762d18706280747d1f97R102 |
Another (slightly more obscure) example:
|
I checked all uses of
con <- simulate_dbi()
translate_sql(x[1L], con = con)
translate_sql(desc(1), con = con)
translate_sql(n_distinct(1), con = con) We can fix them but is there a valid use case for them?
|
Note that |
Thank you, great call! Leaving (2) aside seems fine to me. |
Problem summary
In #1228, most references to
build_sql()
where replaced withglue_sql2()
.This subtly broke some fundamental SQL translations, since
escape()
methods were no longer being applied.Reprex (using Snowflake as my motivating example)
Previously:
current translation, which fails:
Solutions considered
escape(x)
in applicable calls toglue_sql2()
(e.g., add escaping for Snowflake interval functions #1350 to mitigate breaking Snowflake translations)escape()
functionality frombuild_sql()
togluesql2()
(https://github.com/tidyverse/dbplyr/blob/main/R/build-sql.R#L32-L44)The text was updated successfully, but these errors were encountered: