Postgresql #1
Postgresql #1
Conversation
DESCRIPTION
Outdated
| @@ -13,12 +13,15 @@ Description: Cache the results of a function so that when you call it | |||
| URL: https://github.com/r-lib/memoise | |||
| BugReports: https://github.com/r-lib/memoise/issues | |||
| Imports: | |||
| digest (>= 0.6.3) | |||
| digest (>= 0.6.3), | |||
R/cache_postgresql.R
Outdated
| if (!(requireNamespace("readr"))) { stop("Package `readr` must be installed for `cache_postgresql()`.") } # nocov | ||
|
|
||
| cache_reset <- function() { | ||
| dbSendQuery( |
There was a problem hiding this comment.
The tests are failing from R packaging, e.g. DBI::dbSendQuery needs to be added to all the imported functions used
R/cache_postgresql.R
Outdated
| temp_file <- file.path(path, key) | ||
| on.exit(unlink(temp_file)) | ||
| saveRDS(value, file = temp_file, compress = compress, ascii = TRUE) | ||
| encoded <- readr::read_file(temp_file) |
There was a problem hiding this comment.
Why is the readr dependency necessary? Would readBin from base do?
R/cache_postgresql.R
Outdated
| key = key | ||
| ) | ||
| ) | ||
| readr::write_file(rs[1][[1]], temp_file) |
There was a problem hiding this comment.
Is it possible to read the file directly from rs instead of writing to/from temporary files?
There was a problem hiding this comment.
My answer is no, I don't think you can serialize without using textConnections, which have memory limits.
R/cache_postgresql.R
Outdated
| ) | ||
| is_here <- nrow(rs) == 1 | ||
| # if not result is logical(0) | ||
| if(identical(is_here, logical(0))){ |
There was a problem hiding this comment.
if (!is.null(rs) && nrow(rs) == 1) TRUE else FALSE will do it one line
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
=========================================
Coverage ? 68.37%
=========================================
Files ? 6
Lines ? 313
Branches ? 0
=========================================
Hits ? 214
Misses ? 99
Partials ? 0
Continue to review full report at Codecov.
|
|
Back to you! |
|
This is as good as I can review! LGTM |
|
Thanks for looping me in too, I learned a lot! |
No description provided.