Make pool compatible with dbplyr

[bborgesr]

Fixes #39

# install.packages("nycflights13")
# install.packages("dplyr")
# devtools::install_github("rstudio/pool@fix/dbplyr")

library(dplyr)
library(pool)

# Using dbplyr directly

# con <- DBI::dbConnect(RSQLite::SQLite(), path = ":memory:")
# 
# copy_to(con, nycflights13::flights, "flights",
#   temporary = FALSE, 
#   indexes = list(
#     c("year", "month", "day"), 
#     "carrier", 
#     "tailnum",
#     "dest"
#   )
# )
# 
# (flights_db <- tbl(con, "flights"))
# 
# flights_db %>% select(year:day, dep_delay, arr_delay)
# flights_db %>% filter(dep_delay > 240)
# flights_db %>% 
#   group_by(dest) %>%
#   summarise(delay = mean(dep_time))
# 
# tailnum_delay_db <- flights_db %>% 
#   group_by(tailnum) %>%
#   summarise(
#     delay = mean(arr_delay),
#     n = n()
#   ) %>% 
#   arrange(desc(delay)) %>%
#   filter(n > 100)
# 
# tailnum_delay_db


# Using pool
pool <- dbPool(RSQLite::SQLite(), path = ":memory:")

copy_to(pool, nycflights13::flights, "flights",
  temporary = FALSE, 
  indexes = list(
    c("year", "month", "day"), 
    "carrier", 
    "tailnum",
    "dest"
  )
)

(flights_db <- tbl(pool, "flights"))

flights_db %>% select(year:day, dep_delay, arr_delay)
flights_db %>% filter(dep_delay > 240)
flights_db %>% 
  group_by(dest) %>%
  summarise(delay = mean(dep_time))

tailnum_delay_db <- flights_db %>% 
  group_by(tailnum) %>%
  summarise(
    delay = mean(arr_delay),
    n = n()
  ) %>% 
  arrange(desc(delay)) %>%
  filter(n > 100)

tailnum_delay_db

poolClose(pool)

[hadley]

I think this would be slightly better as checkTemporary(), i.e. including the boilerplate if (temporary) stop(temporaryErrorMessage)

[jcheng5]

@hadley What do you think of long, narrative error messages like this?

[hadley]

I have mixed feelings. They feel inelegant to me, but users tend to really appreciate them.

[hadley]

I do wonder about changing the default value of temporary here. Obviously it's going to be annoying if it's TRUE, but it might be surprising if it's FALSE.

[jcheng5]

Isn't the default always FALSE? Are you wondering about changing the default in dplyr itself?

[bborgesr]

You mean changing it in the original generic in dplyr? Or just here in pool?

[bborgesr]

(sorry Joe, didn't see your comment asking exactly the same thing)

[hadley]

Isn't the default in the generic TRUE?

[bborgesr]

@hadley No: https://github.com/tidyverse/dplyr/blob/master/R/dbplyr.R#L95 Or am I missing something?

[hadley]

Ok, I was confused. You can ignore this whole thread.

[hadley]

I now know how to convert

db_con <- poolCheckout(dest)
on.exit(poolReturn(db_con))

to

db_con <- localPoolCheckout(dest)

with the on.exit() being automatically added.

It's up to you if you'd prefer that API.

[jcheng5]

I find that slightly terrifying

[bborgesr]

If it's quick to explain, can you elaborate on why, @jcheng5? Also, @hadley, this is merely in your head now, right? (I couldn't find any instances of localPoolCheckoutin github)

[bborgesr]

@hadley, is there anything that you think I may have missed? All I did was a thin Pool wrapper around all the methods for DBIConnection (including copy_to() and tbl()), and throw an error when temporary = TRUE. Is this enough? In particular, I was wondering about your 3rd item from here:

Rework joins so that upload happens later, using same connection as the rest of the query

[bborgesr]

another worry I have is if this function is amenable to pool. Is the name = random_table_name()'s value actually made into a table using create_table or something?

[hadley]

No, it's for databases that require subqueries be named (even if that name isn't actually used for anything).

[hadley]

The 3rd item is optional, so I wouldn't worry about it for now.

You should probably also include some basic tests of pool dplyr using RSQLite.

[hadley]

No, it's for databases that require subqueries be named (even if that name isn't actually used for anything).

[hadley]

This will create pool in the global environment, which you probably don't want

[hadley]

warning -> error