Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdds Teradata translation #43
Conversation
| @@ -9,6 +9,7 @@ base_odbc_scalar <- sql_translator(.parent = base_scalar, | |||
| as.character = sql_cast("STRING"), | |||
| as.Date = sql_cast("DATE"), | |||
| paste0 = sql_prefix("CONCAT"), | |||
| sd = sql_prefix("STDDEV_SAMP"), | |||
hadley
Oct 23, 2017
Member
Are you sure this is ok to add here?
Are you sure this is ok to add here?
| build_sql( | ||
| "SUBSTR(", x, ", ", start, ", ", len, ")" | ||
| )}, | ||
| paste = sql_not_supported("paste()") |
edgararuiz
Oct 23, 2017
Author
Collaborator
Right, I use CONCAT for paste0. CONCAT does not add a separator like paste does by default.
Line 11
in
d08d930
Right, I use CONCAT for paste0. CONCAT does not add a separator like paste does by default.
Line 11 in d08d930
hadley
Oct 23, 2017
Member
Ah got it. Might be better to have a more helpful error here? i.e. try paste0() instead
Ah got it. Might be better to have a more helpful error here? i.e. try paste0() instead
edgararuiz
Oct 23, 2017
•
Author
Collaborator
Good idea, I'd like to use a function for that, can I add sql_not_supported() a new instead argument? Or should I create a new function?
Good idea, I'd like to use a function for that, can I add sql_not_supported() a new instead argument? Or should I create a new function?
hadley
Oct 24, 2017
Member
I think you can just create a new function inline
I think you can just create a new function inline
| if (isTRUE(all.equal(base, exp(1)))) { | ||
| build_sql("ln(", x, ")") | ||
| } else { | ||
| # Use log change-of-base because postgres doesn't support the |
hadley
Oct 23, 2017
Member
Is this true for teradata too? If so we should pull out into a resuable function
Is this true for teradata too? If so we should pull out into a resuable function
edgararuiz
Oct 23, 2017
Author
Collaborator
Ok, where should I place the function? I'm thinking traslate-sql-helpers.R
Ok, where should I place the function? I'm thinking traslate-sql-helpers.R
hadley
Oct 23, 2017
Member
Yeah, sounds good.
Yeah, sounds good.
|
Ok, I moved PTAL @hadley |
|
|
||
| #' @rdname sql_variant | ||
| #' @export | ||
| sql_formula_log <- function(f, base = exp(1)) { |
hadley
Oct 24, 2017
Member
I think these can just be sql_log and sql_cot, and like sql_cast() etc should return functions
I think these can just be sql_log and sql_cot, and like sql_cast() etc should return functions
| sql("CAST(`field_name` AS VARCHAR(MAX))") | ||
| ) | ||
| }) | ||
| test_that("log10() translates to LOG ", { |
hadley
Oct 24, 2017
Member
Now that we have sql_log() used in multiple places, there's no need to test for each individual driver that uses it.
Now that we have sql_log() used in multiple places, there's no need to test for each individual driver that uses it.
|
|
||
| # Teradata base_agg conversions ----------------------------------------- | ||
|
|
||
| test_that("sd() translates to STDEV ", { |
hadley
Oct 24, 2017
Member
Can you please bundle these all into one test, writing a function as needed to reduce duplication?
Can you please bundle these all into one test, writing a function as needed to reduce duplication?
|
PTAL @hadley |
This change includes the new Teradata translation, and its corresponding tests.
The new translations was tested using
dbtest, here are the results:Fixes: tidyverse/dplyr#3040