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

Postgres: substring generates numeric arguments instead of integer #378

Closed
dfrankow opened this issue Nov 29, 2019 · 10 comments
Closed

Postgres: substring generates numeric arguments instead of integer #378

dfrankow opened this issue Nov 29, 2019 · 10 comments
Labels
bug an unexpected problem or unintended behavior func trans 🌍 Translation of individual functions to SQL

Comments

@dfrankow
Copy link

Brief description of the problem:

library(dbplyr)

translate_sql(substring('abcdefghijklmno', 6,7))
#> Expected: <SQL> substring('abcdefghijklmno', 6, 7)
#> Actual: <SQL> substring('abcdefghijklmno', 6.0, 7.0)

It looks like there was a fix here, but almost two years later, I don't seem to have it. I don't know which version the fix went into.

Maybe I have an old version?

> packageVersion("dbplyr")
[1] ‘1.4.2’
> packageVersion("tidyverse")
[1] ‘1.3.0’

I tried dbplyr from the vanilla tidyverse install (1.3.0), then I also tried installing dbplyr from github:

devtools::install_github("tidyverse/dbplyr")

In both cases it looks like the numbers become float (numeric), and that makes Postgres (11) cranky in some other code I have:

Error in postgresqlExecStatement: RS-DBI driver: (could not Retrieve the result : ERROR:  function pg_catalog.substring(text, numeric, numeric) does not exist.
@dfrankow
Copy link
Author

FYI I was trying to use this to make up rounded dates: floor_date didn't work, I tried substring(1,7)+'-01'). I ended up working around this with:

          started_at = date_trunc("year", started_at))

which worked in dbplyr.

So while this is still an issue, it is not a blocker for me.

@hadley

This comment has been minimized.

@hadley hadley added the reprex needs a minimal reproducible example label Dec 13, 2019
@dfrankow

This comment has been minimized.

@hadley hadley added bug an unexpected problem or unintended behavior func trans 🌍 Translation of individual functions to SQL and removed reprex needs a minimal reproducible example labels Apr 14, 2020
@hadley
Copy link
Member

hadley commented Apr 14, 2020

@dfrankow sorry, must've been in rush and I didn't read your issue fully — your initial description was fine.

@hadley hadley changed the title pg_catalog.substring arguments numeric instead of integer Postgres: substring generates numeric arguments instead of integer Sep 17, 2020
@hadley
Copy link
Member

hadley commented Sep 22, 2020

The problem is that you're using substring() which isn't translated because it's not an R function. You want to use the R function substr():

library(dbplyr)

translate_sql(substring('abcdefghijklmno', 6,7))
#> <SQL> substring('abcdefghijklmno', 6.0, 7.0)
translate_sql(substr('abcdefghijklmno', 6,7))
#> <SQL> SUBSTR('abcdefghijklmno', 6, 2)

Created on 2020-09-22 by the reprex package (v0.3.0.9001)

@hadley hadley closed this as completed Sep 22, 2020
@dfrankow
Copy link
Author

Thanks for the reply!

I don't understand "substring is not an R function". It looks like an R function to my naive eyes (see below). Maybe you mean it's not an internal function.

However, this work-around works for me. You don't have to reply.

Substring docs here.

Code saying it's in base:

> substring
function (text, first, last = 1000000L) 
{
    if (!is.character(text)) 
        text <- as.character(text)
    n <- max(lt <- length(text), length(first), length(last))
    if (lt && lt < n) 
        text <- rep_len(text, length.out = n)
    .Internal(substr(text, as.integer(first), as.integer(last)))
}
<bytecode: 0x7f91b92399a0>
<environment: namespace:base>

@hadley
Copy link
Member

hadley commented Sep 22, 2020

Oh shit, you're right. I got confused again because I never use substring(). I'll add it as an additional translation to all the backends.

@hadley hadley reopened this Sep 22, 2020
@dfrankow
Copy link
Author

As you like. Thanks for the tidyverse!

@hadley hadley closed this as completed in d826fbf Sep 22, 2020
@hadley
Copy link
Member

hadley commented Sep 22, 2020

Sorry it took so longer to get this through my head!

@dfrankow
Copy link
Author

Thanks!

I don't want you to do more work, but just so I remember: substr and substring are not quite the same.

The "last" param for substring is optional, while the "stop" param for substr is not. Since d826fbf uses the substr implementation, it may force the optional "last" to be given. Doesn't bother me.

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
Projects
None yet
Development

No branches or pull requests

2 participants