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

explain for a SQL query is affected by options(digits.secs) #1440

Closed
chicotobi opened this issue Jan 11, 2024 · 1 comment
Closed

explain for a SQL query is affected by options(digits.secs) #1440

chicotobi opened this issue Jan 11, 2024 · 1 comment

Comments

@chicotobi
Copy link

chicotobi commented Jan 11, 2024

Expectation: Copy&Paste of the explain output into the SQL CLI should yield the same result as executing the query.
But here the output of explain depends on options(digits.secs). Is that correct?

rm(list=ls())

library(RSQLite)
library(dplyr)
library(dbplyr)

# Create example table in-memory
con <- dbConnect(RSQLite::SQLite(), ":memory:", extended_types = TRUE)
con %>% dbExecute("CREATE TABLE tmp (t datetime );")
time1 <- "2024-01-11 09:17:12.345" %>% as.POSIXct()
df <- data.frame(t=time1)
con %>% dbAppendTable("tmp",df)

# Query with a datetime filter - digits.secs is 0, result is not affected
time2 <- "2024-01-11 09:17:12.678" %>% as.POSIXct()
options(digits.secs=0)
con %>% tbl("tmp") %>% filter(t<=time2) %>% explain()
con %>% tbl("tmp") %>% filter(t<=time2) %>% collect()

# Query with a datetime filter - digits.secs is 3
options(digits.secs=3)
con %>% tbl("tmp") %>% filter(t<=time2) %>% explain()
con %>% tbl("tmp") %>% filter(t<=time2) %>% collect()

Output:

<SQL>
SELECT *
FROM `tmp`
WHERE (`t` <= '2024-01-11T08:17:12Z')

<PLAN>
  id parent notused   detail
1  2      0       0 SCAN tmp
<SQL>
SELECT *
FROM `tmp`
WHERE (`t` <= '2024-01-11T08:17:12.677Z')

<PLAN>
  id parent notused   detail
1  2      0       0 SCAN tmp
@DavisVaughan DavisVaughan transferred this issue from tidyverse/dplyr Jan 12, 2024
@hadley
Copy link
Member

hadley commented Feb 14, 2024

That's coming from this code: https://github.com/tidyverse/dbplyr/blob/main/R/db-escape.R#L49-L53

So it seems probably fine to me? I don't love that a global option affects this, but that option is baked fairly deeply in to R itself, and if that option didn't exist, we'd have to find some other way to pass the number of digits.

@hadley hadley closed this as completed Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants