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

NULL %foo% lhs is not translated to foo lhs anymore ? #1345

Closed
moodymudskipper opened this issue Aug 7, 2023 · 6 comments · Fixed by #1457
Closed

NULL %foo% lhs is not translated to foo lhs anymore ? #1345

moodymudskipper opened this issue Aug 7, 2023 · 6 comments · Fixed by #1457
Labels
feature a feature request or enhancement func trans 🌍 Translation of individual functions to SQL
Milestone

Comments

@moodymudskipper
Copy link

moodymudskipper commented Aug 7, 2023

I had the following code and it worked ok (not sure with which version):

data |>
  mutate(year = extract(NULL %year from% mydate))

Now it is broken, I could fix by changing to:

data |>
  mutate(year = extract(`year from`(mydate)))

The translation is extract(year from("mydate").

I'm not sure if the previous behavior was documented. I don't see the breaking change idocumented in the NEWS. Was it purposefully disabled ? I thought it was neat.

Note: I just found out that I could just use year() (on Oracle), not sure if it's new or not, but tangential to the issue.

@mgirlich
Copy link
Collaborator

mgirlich commented Aug 8, 2023

So you mean that previously extract(NULL %year from% mydate) was translated to extract(year from `mydate`)? Interesting, I don't think this was intended nor was it intended to change, therefore there is no breaking change documented.

I see that it would be nice to somehow get the SQL code extract(year from `mydate`) but I'm not quite convinced of NULL %year from% mydate.

@mgirlich mgirlich added this to the 2.5.0 milestone Oct 27, 2023
@hadley
Copy link
Member

hadley commented Nov 2, 2023

This works, but is a bit clunky:

library(dbplyr)
translate_sql(extract(sql("year") %from% 'mydate'), con = simulate_postgres())
#> <SQL> extract(year from 'mydate')

Created on 2023-11-02 with reprex v2.0.2

Maybe we could provide built-in %from% and %as% infix helpers for these common cases?

@hadley hadley added feature a feature request or enhancement func trans 🌍 Translation of individual functions to SQL labels Nov 2, 2023
@krlmlr
Copy link
Member

krlmlr commented Feb 15, 2024

I remember using this feature extensively in a project, and I have it on my workshop slides somewhere.

@krlmlr
Copy link
Member

krlmlr commented Feb 15, 2024

Can we just pass through %whatever% unchanged?

@hadley
Copy link
Member

hadley commented Feb 15, 2024

Since this is a regression we can just fix it for 2.4.1. I don't love the syntax, but it works, and there's no obviously better alternative.

@hadley hadley modified the milestones: 2.5.0, 2.4.1 Feb 15, 2024
@hadley
Copy link
Member

hadley commented Feb 20, 2024

Most likely root cause: https://github.com/tidyverse/dbplyr/pull/1228/files#diff-bffb8f93561affc007b200e783a30fb6b418d48ffa60175ade2a2fc3818bc532

Reprex:

library(dbplyr)
translate_sql(extract(NULL %year from% mydate), con = simulate_postgres())
#> <SQL> extract(NULL year from `mydate`)

Created on 2024-02-20 with reprex v2.0.2.9000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement func trans 🌍 Translation of individual functions to SQL
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants