From 48ccd34bef8b85ed7b6cf5e90b1ca5a7a2d6ab72 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Wed, 22 Dec 2021 11:06:43 -0800 Subject: [PATCH 01/38] Fix dolt_push set_upstream (#36) --- R/dolt-remote.R | 12 ++++++++---- man/dolt-remote.Rd | 3 +++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/R/dolt-remote.R b/R/dolt-remote.R index 6eb469c..8685c4c 100644 --- a/R/dolt-remote.R +++ b/R/dolt-remote.R @@ -1,6 +1,7 @@ #' Work with dolt repository remotes #' #' @param remote the name of the remote. "origin" is used by default +#' @param remote_branch the name of the remote branch to use with set_upstream. Current local branch is used by default #' @param ref the branch reference #' @param set_upstream whether to set the remote branch reference to track #' @param force whether to overwrite any conflicting history @@ -10,14 +11,17 @@ #' @rdname dolt-remote #' @family dolt-sql-commands #' @importFrom dbplyr sql_quote -dolt_push <- function(remote = NULL, ref = NULL, set_upstream = FALSE, - force = FALSE, conn = dolt(), collect = NULL, - show_sql = NULL) { +dolt_push <- function(remote = NULL, remote_branch = NULL, ref = NULL, + set_upstream = FALSE, force = FALSE, conn = dolt(), + collect = NULL, show_sql = NULL) { collect <- .collect(collect); show_sql <- .show_sql(show_sql) args <- character(0) + if (set_upstream & is.null (remote)) remote = "origin" + if (set_upstream & is.null (remote_branch)) remote_branch = sub(".*/", "", dolt_state()$head_ref) if (!is.null (remote)) args <- c(args, sql_quote(remote, "'")) + if (!is.null (remote_branch)) args <- c(args, sql_quote(remote_branch, "'")) if (!is.null (ref)) args <- c(args, sql_quote(ref, "'")) - if (set_upstream) args <- c("'--set-upstream'", args) + if (set_upstream) args <- c("'--set-upstream' ", args) if (force) args <- c(args, "'--force'") query <- paste0("select dolt_push(", paste0(args, collapse = ", "), ")") dolt_query(query, conn, collect, show_sql) diff --git a/man/dolt-remote.Rd b/man/dolt-remote.Rd index 69b1ca1..a7749c7 100644 --- a/man/dolt-remote.Rd +++ b/man/dolt-remote.Rd @@ -9,6 +9,7 @@ \usage{ dolt_push( remote = NULL, + remote_branch = NULL, ref = NULL, set_upstream = FALSE, force = FALSE, @@ -44,6 +45,8 @@ dolt_clone( \arguments{ \item{remote}{the name of the remote. "origin" is used by default} +\item{remote_branch}{the name of the remote branch to use with set_upstream. Current local branch is used by default} + \item{ref}{the branch reference} \item{set_upstream}{whether to set the remote branch reference to track} From 112acb5cb79bf29908fd8af4ff00f90915abb43b Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Wed, 16 Mar 2022 09:42:55 -0400 Subject: [PATCH 02/38] Remove use of R 4.1+ lambda syntax --- R/dolt-types.R | 2 +- doltr.Rproj | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/R/dolt-types.R b/R/dolt-types.R index 16bb440..49aa9c6 100644 --- a/R/dolt-types.R +++ b/R/dolt-types.R @@ -80,7 +80,7 @@ dolt_text_type <- function(obj, min_varchar, max_varchar) { #' @importFrom blob as_blob dolt_blob_type <- function(obj) { if (!all(vapply(obj, is.raw, logical(1)))) "Stop only lists of raw vectors (blobs) allowed" - nb <- max(vapply(obj, \(x) length(x), 1), 1, na.rm = TRUE) + nb <- max(vapply(obj, length, 1), 1, na.rm = TRUE) if (nb <= 65535) { return(structure("BLOB", max_size = 65535)) } else if (nb > 65535L && nb <= 16777215) { diff --git a/doltr.Rproj b/doltr.Rproj index 517abe7..70ecf7a 100644 --- a/doltr.Rproj +++ b/doltr.Rproj @@ -17,5 +17,6 @@ StripTrailingWhitespace: Yes BuildType: Package PackageUseDevtools: Yes +PackageCleanBeforeInstall: Yes PackageInstallArgs: --no-multiarch --with-keep.source PackageRoxygenize: rd,collate,namespace From 65035e4b42f5f4301ece984142cc8c4bb23da266 Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Fri, 13 May 2022 12:12:20 -0400 Subject: [PATCH 03/38] Ignore .sqlhistory --- R/utils.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/utils.R b/R/utils.R index ead9bce..5b8109e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -121,3 +121,8 @@ as_table <- function(schema, table) { args <- args[!is.na(args) & args != ""] do.call(Id, as.list(args)) } + +compact <- function(x) { + is_empty <- vapply(x, function(x) length(x) == 0, logical(1)) + x[!is_empty] +} From 77ed2f69331eab24112acb17f51a9ec7180c333c Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Fri, 13 May 2022 12:12:57 -0400 Subject: [PATCH 04/38] Ignore .sqlhistory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index cc7c3d2..0f6e5e8 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ docs tests/testthat/doltdb inst/doc dolt +.sqlhistory From d3b6791dbc77b7c21498919eb0c6665317b5f48c Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Fri, 13 May 2022 14:57:38 -0400 Subject: [PATCH 05/38] Kill server not responding to queries as invalid --- R/dolt-local-connection.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index 8195e7e..d1ba3ea 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -159,6 +159,7 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { #' @rdname dolt_local setMethod("dbIsValid", "DoltLocalConnection", function(dbObj, ...) { valid <- getMethod(dbIsValid, "MariaDBConnection")(dbObj) && + class(try(dbGetQuery(dbObj, "SELECT 1"), silent = TRUE)) != "try-error" && ps_is_running(dbObj@server) if (!valid && inherits(dbObj@server, "ps_handle")) try(dkill(dbObj@server), silent = TRUE) From 55532cfbd3b855d1ded3e9439cb996b3987400d2 Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Thu, 19 May 2022 09:23:26 -0400 Subject: [PATCH 06/38] Add vignette --- vignettes/doltr.Rmd | 182 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 vignettes/doltr.Rmd diff --git a/vignettes/doltr.Rmd b/vignettes/doltr.Rmd new file mode 100644 index 0000000..7c6ec98 --- /dev/null +++ b/vignettes/doltr.Rmd @@ -0,0 +1,182 @@ +--- +title: "Getting Started with Doltr" +output: rmarkdown::html_vignette +vignette: > + %\VignetteIndexEntry{Getting Started with Doltr} + %\VignetteEngine{knitr::rmarkdown} + %\VignetteEncoding{UTF-8} +--- + +```{r, include = FALSE} +knitr::opts_chunk$set( + collapse = TRUE, + comment = "#>" +) +``` + +`{doltr}` is a package to interface with [Dolt](https://www.dolthub.com), an +SQL database with git-like versioning. + +# Installation + +You will need the **dolt** command-line utility installed on your computer to use `{doltr}`. Installation instructions +for Linux, macOS, and Windows can be found [here](https://docs.dolthub.com/getting-started/installation). + +Install the R package with + +``` r +remotes::install_github("ecohealthalliance/doltr") +``` + +# Usage + +`{doltr}` package provides two [DBI-compliant drivers](https://github.com/r-dbi/DBI#dbi) +to connect to a dolt database `dolt_remote()` connects to a dolt server via TCP, +and is a thin wrapper around the [RMariaDB](https://rmariadb.r-dbi.org/) package +because Dolt shares a communication protocol with MySQL and MariaDB. +`dolt_local()` connects to a Dolt database directory locally on-disk. Behind the +scenes `dolt_local()` launches and manages a background server process, which +can also be done manually with `dolt_server()`. Both can be used to connect to +a database as you would with other DBI packages: + + +``` +library(doltr) +remote_conn <- DBI::dbConnect(dolt_remote(), dname = "dbname", username = "user", ...) +``` + +``` +local_conn <- DBI::dbConnect(dolt_local(), dir = "/path/to/my/dolt/db/directory") +``` + +Since Dolt has git-like versioning concepts, `{doltr}`'s API design includes +both components drawn from `{DBI}` and also from git interfaces like `{gert}` +and `{git2r}` (as well as Dolt's [command-line interface](https://docs.dolthub.com/interfaces/cli)). + + +`{doltr}` has the concept of a "default database" for a project. When +working with git (or `{git2r}` or `{gert}`), commands apply to the current +working directory by default. Similarly, with `{doltr}`, many commands use +the default database. The default database is set with the environment +directory `DOLT_DIR`, which is `doltdb` by default. +For a project you might set `DOLT_DIR` in a project-level `.Renviron` or +[`.env` file](https://cran.r-project.org/web/packages/dotenv/) + +To explore `{doltr}`'s capabilities, let's pull an existing database. `dolt_clone()`, +like `git clone` clones a database to a local directory, using [`DoltHub`](https://www.dolthub.com/discover) +as the default remote source (though [dolt Database remotes can be hosted elsewhere](https://www.dolthub.com/discover)). +We'll clone [`doltr/nycflights`](https://www.dolthub.com/repositories/doltr/nycflights), which contains a subset of +the data from the [{`nycflights13`} package](https://nycflights13.tidyverse.org/). + +```{r cleanfirst, include = FALSE} +if (dir.exists("nycflights")) unlink("nycflights", force = TRUE, recursive = TRUE) +``` + +```{r clone-flights} +library(doltr) +dolt_clone("doltr/nycflights") +``` + +This creates an `nycflights` directory. Let's set it as our default database for +this session: + +```{r setenv} +Sys.setenv(DOLT_DIR="nycflights") +``` + +You can use the `dolt()` function to connect to the database. `dolt()` is a +shortcut for `dbConnect(dolt_local/dolt_remote(), ...)`. It also caches the database +connection, so it can be called repeatedly in place of a connection variable. +`dolt()` is also the default argument for a database connection in many functions. + +Running `dolt()` prints a summary of the database state: + +```{r doltcmd} +dolt() +``` + +You can use `dolt()` with `{DBI}` or `{dbplyr}` functions to read from or write +to the database: + +```{r pkgs, message=FALSE} +library(DBI) +library(dbplyr) +library(dplyr) +``` + +```{r dbi} +dbListTables(dolt()) +dbReadTable(dolt(), "airlines") +``` + +```{r dbplyr} +tbl(dolt(), "flights") %>% + filter(origin == "EWR", dest == "MDW") %>% + head() %>% + collect() +``` + +```{r writetbl} +dbWriteTable(dolt(), "mtcars", mtcars) +``` + +With the last command, we changed the database by adding a table. This is +reflected in the change to the database working state when we print `dolt()` + +```{r state} +dolt() +``` + +The summary no longer says "Working database clean" but shows that the _working state_ +of the database now includes a new table called `mtcars`. As with a new file in +a git repository, we can _stage_ this table for comitting, with `dolt_add()`. +Rather than printing the whole database summary, we can get just the last +bullet with `dolt_status()` + +```{r} +dolt_add("mtcars") +dolt_status() +``` + +`dolt_status()` pretty-prints but actually yields a table of working or stages +changes to the database: + +```{r} +as.data.frame(dolt_status()) +``` + +With the table staged, we can commit it to the database and provide a message: + +```{r} +dolt_commit(message = "Add mtcars table") +dolt_status() +dolt_last_commit() +``` + +# Exploring Dolt history + +You can view the commit history of the database with `dolt_log()`, which collects +the Dolt system table `dolt_log`: + +```{r} +dolt_log() +``` + +# + +## The Connection Pane + +For RStudio users, `{doltr}` provides a [connection pane](https://db.rstudio.com/tooling/connections/) with which you can explore the database. + + +```{r, eval=FALSE} +dolt_pane() +``` + +`{doltr}`'s connection pane shows information about the versioning state of your +database in addition to your tables, the Dolt system tables and the database +information schema. + + + +[![Created by EcoHealth Alliance](figures/eha-footer.png)](https://www.ecohealthalliance.org/) From 8d1b14eb7ad92e3333de7048057cae7f7ca34132 Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Thu, 19 May 2022 14:43:55 -0400 Subject: [PATCH 07/38] Fix dolt_diffs() --- DESCRIPTION | 2 +- R/dolt-diffs.R | 6 ++++-- man/dolt-diffs.Rd | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4443d27..61a13ce 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -45,7 +45,7 @@ VignetteBuilder: Encoding: UTF-8 Language: en-US Roxygen: list(markdown = TRUE) -RoxygenNote: 7.1.2 +RoxygenNote: 7.2.0 SystemRequirements: dolt Collate: 'cli.R' diff --git a/R/dolt-diffs.R b/R/dolt-diffs.R index 3f310f5..038054c 100644 --- a/R/dolt-diffs.R +++ b/R/dolt-diffs.R @@ -2,12 +2,14 @@ #' #' @param table [character] the name of a table in the database #' @param to commit to compare to +#' @param from commit to compare from #' @inheritParams dolt_branches #' @export #' @rdname dolt-diffs -dolt_diffs <- function(table, to, conn = dolt(), collect = NULL, show_sql = NULL) { +dolt_diffs <- function(table, to, from, conn = dolt(), collect = NULL, show_sql = NULL) { collect <- .collect(collect); show_sql <- .show_sql(show_sql) - query <- paste0("select * from dolt_commit_diff_", table, " WHERE to_commit = ", to) + query <- paste0("select * from dolt_commit_diff_", table, + " where to_commit='", to, "' and from_commit='", from, "'") dolt_query(query, conn, collect, show_sql) } diff --git a/man/dolt-diffs.Rd b/man/dolt-diffs.Rd index a21b2de..e5c4aaf 100644 --- a/man/dolt-diffs.Rd +++ b/man/dolt-diffs.Rd @@ -5,7 +5,7 @@ \alias{dolt_table_history} \title{Examine information about dolt tables and diffs} \usage{ -dolt_diffs(table, to, conn = dolt(), collect = NULL, show_sql = NULL) +dolt_diffs(table, to, from, conn = dolt(), collect = NULL, show_sql = NULL) dolt_table_history(table, conn = dolt(), collect = NULL, show_sql = NULL) } @@ -14,6 +14,8 @@ dolt_table_history(table, conn = dolt(), collect = NULL, show_sql = NULL) \item{to}{commit to compare to} +\item{from}{commit to compare from} + \item{conn}{the database connection} \item{collect}{whether to collect the result into R or return a \code{\link[dbplyr:tbl_lazy]{dbplyr::tbl_lazy()}} From bb068af3624c2d93acf4014b60384b69a02f7f0b Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Thu, 19 May 2022 18:12:57 -0400 Subject: [PATCH 08/38] Add parquet export --- R/cli.R | 5 +++-- man/dolt_dump.Rd | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/R/cli.R b/R/cli.R index e95ee43..a8b1211 100644 --- a/R/cli.R +++ b/R/cli.R @@ -40,7 +40,8 @@ dolt_init <- function(dir = Sys.getenv("DOLT_DIR", "doltdb")) { #' Export data from a dolt database #' @param dir path to dolt database on-disk -#' @param format the export data format. One of `"sql"`, `"csv"`, or `"json"` +#' @param format the export data format. One of `"sql"`, `"csv"`, `"json"`, or +#' `"parquet"` #' @param out the location on-disk for export. In the case of `"sql"`, format, #' a single file path (default `doltdump.sql`), otherwise a directory for all #' tables to be dumped as separate files (default "doltdump") @@ -49,7 +50,7 @@ dolt_init <- function(dir = Sys.getenv("DOLT_DIR", "doltdb")) { #' @importFrom R.utils getAbsolutePath #' @return the path(s) of exported files #' @export -dolt_dump <- function(format = c("sql", "csv", "json"), +dolt_dump <- function(format = c("sql", "csv", "json", "parquet"), out = NULL, overwrite = FALSE, dir = Sys.getenv("DOLT_DIR", "doltdb")) { diff --git a/man/dolt_dump.Rd b/man/dolt_dump.Rd index 7abdd91..30aea17 100644 --- a/man/dolt_dump.Rd +++ b/man/dolt_dump.Rd @@ -5,14 +5,15 @@ \title{Export data from a dolt database} \usage{ dolt_dump( - format = c("sql", "csv", "json"), + format = c("sql", "csv", "json", "parquet"), out = NULL, overwrite = FALSE, dir = Sys.getenv("DOLT_DIR", "doltdb") ) } \arguments{ -\item{format}{the export data format. One of \code{"sql"}, \code{"csv"}, or \code{"json"}} +\item{format}{the export data format. One of \code{"sql"}, \code{"csv"}, \code{"json"}, or +\code{"parquet"}} \item{out}{the location on-disk for export. In the case of \code{"sql"}, format, a single file path (default \code{doltdump.sql}), otherwise a directory for all From 5bcf02d1b93411bba7d5cba33a96edfa418d6db4 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Thu, 9 Jun 2022 12:16:06 -0700 Subject: [PATCH 09/38] Add return value to query_as_of in read-table.R --- R/read-table.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/read-table.R b/R/read-table.R index 8ba8478..789974e 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -60,6 +60,7 @@ query_as_of <- function(query, as_of) { error = function(e) paste("'", as_of, "'") ) query <- paste0(query, " AS OF ", as_of) + query } #' @export From c81c6178d192d639c8ce082df5d4c056910dbc31 Mon Sep 17 00:00:00 2001 From: Noam Ross Date: Mon, 20 Jun 2022 10:08:10 -0400 Subject: [PATCH 10/38] Fis server timeout --- R/server.R | 13 +++++++++---- man/dolt_server.Rd | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/R/server.R b/R/server.R index 8e2f6fd..a11b7df 100644 --- a/R/server.R +++ b/R/server.R @@ -20,7 +20,7 @@ #' to `std_out()`, if `NULL` (default), it is suppressed. Can also take #' a filename. See [processx::run()]. #' @param timeout Defines the timeout, in seconds, used for connections -#' A value of `0` represents an infinite timeout (default `28800000`) +#' (default `28800000`) #' @param query_parallelism Set the number of go routines spawned to handle each #' query (default `2`) #' @param max_connections Set the number of connections handled by the server @@ -44,7 +44,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), read_only = FALSE, log_level = "info", log_out = NULL, - timeout = 0, + timeout = 28800000, query_parallelism = 2, max_connections = 100, config_file = Sys.getenv("DOLT_CONFIG_FILE", "")) { @@ -175,6 +175,11 @@ dkill <- function(p = ps_handle) { } invisible(NULL) } - - +# +# is_dolt_server_valid <- function(srv) { +# dbConnect(dolt_remote(), dbname = basename(ps_cwd(srv)), username = username, +# password = password, host = host, port = ps_connections(srv)$lport, +# autocommit = autocommit, ...) +# } +# diff --git a/man/dolt_server.Rd b/man/dolt_server.Rd index 531013d..6b7987e 100644 --- a/man/dolt_server.Rd +++ b/man/dolt_server.Rd @@ -17,7 +17,7 @@ dolt_server( read_only = FALSE, log_level = "info", log_out = NULL, - timeout = 0, + timeout = 28800000, query_parallelism = 2, max_connections = 100, config_file = Sys.getenv("DOLT_CONFIG_FILE", "") @@ -59,7 +59,7 @@ to \code{std_out()}, if \code{NULL} (default), it is suppressed. Can also take a filename. See \code{\link[processx:run]{processx::run()}}.} \item{timeout}{Defines the timeout, in seconds, used for connections -A value of \code{0} represents an infinite timeout (default \code{28800000})} +(default \code{28800000})} \item{query_parallelism}{Set the number of go routines spawned to handle each query (default \code{2})} From 0c10c6c4c65180a01b78cd334c1e7c9f9893bc27 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Fri, 29 Jul 2022 12:23:36 -0700 Subject: [PATCH 11/38] Add delay between server process creation and reading --- R/server.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/server.R b/R/server.R index 8e2f6fd..35df23d 100644 --- a/R/server.R +++ b/R/server.R @@ -90,6 +90,8 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), stdout = log_out, stderr = "2>&1", env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) + + Sys.sleep(0.25) # Added delay between process creation and fetching p <- proc$as_ps_handle() rm(proc) @@ -108,6 +110,7 @@ dolt_server_port <- function(p) { #' @importFrom ps ps_pid ps_cwd ps_connections ps_is_running format.dolt_server <- function(x, ...) { pid <- ps_pid(x) + Sys.sleep() if (ps_is_running(x)) { dir <- ps_cwd(x) port <- dolt_server_port(x) From 69e9d2fcd946a7614a856e38b1d00163653ed843 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Fri, 29 Jul 2022 12:24:49 -0700 Subject: [PATCH 12/38] Add diagnostic step to print process id after first read from proc --- R/server.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/server.R b/R/server.R index 35df23d..35ba942 100644 --- a/R/server.R +++ b/R/server.R @@ -93,6 +93,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), Sys.sleep(0.25) # Added delay between process creation and fetching p <- proc$as_ps_handle() + print(p) rm(proc) while(!isTRUE(port %in% ps_connections(p)$lport)) Sys.sleep(0.25) From 1256fbde8eb68ea6563445684512f5875a2eba43 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Fri, 29 Jul 2022 13:15:30 -0700 Subject: [PATCH 13/38] Add looped tryCatch. It will keep waiting until process id is available or 5 minutes goes by after which it will error out --- R/server.R | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/R/server.R b/R/server.R index 35ba942..d42729b 100644 --- a/R/server.R +++ b/R/server.R @@ -91,8 +91,23 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) - Sys.sleep(0.25) # Added delay between process creation and fetching - p <- proc$as_ps_handle() + p_set = F + p_start = Sys.time() + while(!p_set) { + p <- tryCatch({ + p_set = T + proc$as_ps_handle() + }, error=function(cond) { + p_set = F + return(NA) + }) + + if(Sys.time() - p_start > lubridate::minutes(5)) { + p_set = T + error("proc$as_ps_handle() never returned process id") + } + Sys.sleep(0.25) + } print(p) rm(proc) From dc246de5fc030885726ffe7a8faeb16e39b0208f Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Sun, 31 Jul 2022 23:08:59 -0700 Subject: [PATCH 14/38] Update server timeout default to match main --- R/server.R | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/R/server.R b/R/server.R index d42729b..0bb1c6a 100644 --- a/R/server.R +++ b/R/server.R @@ -44,7 +44,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), read_only = FALSE, log_level = "info", log_out = NULL, - timeout = 0, + timeout = 28800000, query_parallelism = 2, max_connections = 100, config_file = Sys.getenv("DOLT_CONFIG_FILE", "")) { @@ -75,6 +75,8 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), paste0("--timeout=", timeout), paste0("--max-connections=", max_connections), paste0("--loglevel=", log_level), + "--net_write_timeout=1800", + "--net_read_timeout=1800", paste0("--query-parallelism=", query_parallelism)) if (password != "") args <- c(args, paste0("--password=", password)) @@ -87,7 +89,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), log_out <- normalizePath(log_out, mustWork = FALSE) proc <- process$new(dolt_path(), args = args, wd = dir, - stdout = log_out, stderr = "2>&1", + stdout = "proc.out", stderr = "proc.err", env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) @@ -126,7 +128,6 @@ dolt_server_port <- function(p) { #' @importFrom ps ps_pid ps_cwd ps_connections ps_is_running format.dolt_server <- function(x, ...) { pid <- ps_pid(x) - Sys.sleep() if (ps_is_running(x)) { dir <- ps_cwd(x) port <- dolt_server_port(x) From be8905ba47c8b8cd13c07699fbc28dc281e179d6 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 07:40:17 -0700 Subject: [PATCH 15/38] Fix net_write_timeout arg --- R/server.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/R/server.R b/R/server.R index 0bb1c6a..578b2b5 100644 --- a/R/server.R +++ b/R/server.R @@ -75,8 +75,6 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), paste0("--timeout=", timeout), paste0("--max-connections=", max_connections), paste0("--loglevel=", log_level), - "--net_write_timeout=1800", - "--net_read_timeout=1800", paste0("--query-parallelism=", query_parallelism)) if (password != "") args <- c(args, paste0("--password=", password)) From 88ef664618de49359b25ab389a5addc71ce44ff7 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 09:27:46 -0700 Subject: [PATCH 16/38] Change dolt_server_find to look for both 'running' and 'sleeping' processes --- R/server.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/server.R b/R/server.R index 578b2b5..9884a18 100644 --- a/R/server.R +++ b/R/server.R @@ -149,7 +149,7 @@ setOldClass("dolt_server") #' @importFrom ps ps ps_connections ps_cwd ps_environ ps_cmdline dolt_server_find <- function(dir = NULL, port = NULL, doltr_only = FALSE) { dp <- ps() - dp <- dp[dp$name == "dolt" & dp$status == "running",] + dp <- dp[dp$name == "dolt" & (dp$status == "running" | dp$status == "sleeping"),] if (nrow(dp)) dp <- dp[vapply(dp$ps_handle, function(x) { isTRUE(try(ps_cmdline(x)[2], silent = TRUE) == "sql-server") From cf6db571d02f6ba619dac23bcf833e50e7b48239 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 10:31:08 -0700 Subject: [PATCH 17/38] Allow dolt_server_find() to find sleeping processes. --- R/server.R | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/R/server.R b/R/server.R index 9884a18..73ff0ae 100644 --- a/R/server.R +++ b/R/server.R @@ -43,7 +43,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), autocommit = TRUE, read_only = FALSE, log_level = "info", - log_out = NULL, + log_out = "proc.log", timeout = 28800000, query_parallelism = 2, max_connections = 100, @@ -87,7 +87,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), log_out <- normalizePath(log_out, mustWork = FALSE) proc <- process$new(dolt_path(), args = args, wd = dir, - stdout = "proc.out", stderr = "proc.err", + stdout = log_out, stderr = "2>&1", env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) @@ -181,6 +181,9 @@ dolt_server_kill <- function(dir = NULL, port = NULL, doltr_only = FALSE, verbos dp <- dolt_server_find(dir, port, doltr_only) lapply(dp$ps_handle, dkill) if (verbose) message(nrow(dp), " processes killed") + + # dolt now uses a lock file which needs to be cleaned up after process killed + # This might be a little complicated with multi_db = T invisible(dp) } @@ -191,8 +194,15 @@ dkill <- function(p = ps_handle) { } else { ps_kill(p) } + invisible(NULL) } - +# +# is_dolt_server_valid <- function(srv) { +# dbConnect(dolt_remote(), dbname = basename(ps_cwd(srv)), username = username, +# password = password, host = host, port = ps_connections(srv)$lport, +# autocommit = autocommit, ...) +# } +# From dd2c7f3012f7a3dbf75f55adf85409e7719ccb70 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 10:32:35 -0700 Subject: [PATCH 18/38] Remove sleep loop waiting for proc to spawn --- R/server.R | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/R/server.R b/R/server.R index 73ff0ae..af492b1 100644 --- a/R/server.R +++ b/R/server.R @@ -90,25 +90,6 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), stdout = log_out, stderr = "2>&1", env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) - - p_set = F - p_start = Sys.time() - while(!p_set) { - p <- tryCatch({ - p_set = T - proc$as_ps_handle() - }, error=function(cond) { - p_set = F - return(NA) - }) - - if(Sys.time() - p_start > lubridate::minutes(5)) { - p_set = T - error("proc$as_ps_handle() never returned process id") - } - Sys.sleep(0.25) - } - print(p) rm(proc) while(!isTRUE(port %in% ps_connections(p)$lport)) Sys.sleep(0.25) From 69c7db8a14af76c812ab1c3252cf62fa97d0ba94 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 10:58:40 -0700 Subject: [PATCH 19/38] Remove default log used in testing --- R/server.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/server.R b/R/server.R index af492b1..907927b 100644 --- a/R/server.R +++ b/R/server.R @@ -43,7 +43,7 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), autocommit = TRUE, read_only = FALSE, log_level = "info", - log_out = "proc.log", + log_out = NULL, timeout = 28800000, query_parallelism = 2, max_connections = 100, From db6cd24582f91d840211b5ea7b04b59c95755b01 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 1 Aug 2022 14:09:05 -0700 Subject: [PATCH 20/38] Deleted line apturing proc$as_ps_handle() getting ready for PR --- R/server.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/server.R b/R/server.R index 907927b..94f80e8 100644 --- a/R/server.R +++ b/R/server.R @@ -90,6 +90,8 @@ dolt_server <- function(dir = Sys.getenv("DOLT_DIR", "doltdb"), stdout = log_out, stderr = "2>&1", env = c("current", R_DOLT=1), supervise = FALSE, cleanup = FALSE, cleanup_tree = FALSE) + + p <- proc$as_ps_handle() rm(proc) while(!isTRUE(port %in% ps_connections(p)$lport)) Sys.sleep(0.25) From e749e512923a5031d872b77d776a58ceb62cf21f Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Tue, 2 Aug 2022 11:25:56 -0700 Subject: [PATCH 21/38] Add lockfile cleanup to dbDisconnect --- R/dolt-local-connection.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index d1ba3ea..7f0b2db 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -151,6 +151,7 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { if (kill_server) try(dkill(conn@server), silent = TRUE) + unlink(paste0(conn@dir, "/.dolt/sql-server.lock")) invisible(TRUE) }) From 6454505fb41944986cb6346e952b051f60be10c8 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Tue, 2 Aug 2022 11:46:32 -0700 Subject: [PATCH 22/38] Move unlink inside try --- R/dolt-local-connection.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index 7f0b2db..e9c0d30 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -136,7 +136,7 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { procs <- procs[procs$status == "running" & procs$pid != ps_pid(ps_handle()),] procs <- procs[vapply(procs$ps_handle, function(x) { conns <- try(ps_connections(x), silent = TRUE) - out <- !inherits(conns, "try-error") && nrow(conns) && conn@port %in% conns$rport + out <- !inherits(conns, "try-error") && nrow(conns) && conn@port %in% conns$lport out }, logical(1)),] other_sessions <- as.logical(nrow(procs)) @@ -149,11 +149,11 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { } getMethod(dbDisconnect, "DoltConnection")(conn) - if (kill_server) - try(dkill(conn@server), silent = TRUE) - unlink(paste0(conn@dir, "/.dolt/sql-server.lock")) - invisible(TRUE) -}) + if (kill_server) { + try({dkill(conn@server) + unlink(paste0(conn@dir, "/.dolt/sql-server.lock")) + }, silent = T) + } #' @export #' @importFrom ps ps_is_running From 92860049a748d6ebd1bef950737abc08eec4bc5a Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Tue, 2 Aug 2022 12:52:09 -0700 Subject: [PATCH 23/38] Try to kill process in a more elegant way that cleans up sql-lock.file automatically. --- R/dolt-local-connection.R | 7 +++---- R/server.R | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index e9c0d30..a4edd7c 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -150,10 +150,9 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { getMethod(dbDisconnect, "DoltConnection")(conn) if (kill_server) { - try({dkill(conn@server) - unlink(paste0(conn@dir, "/.dolt/sql-server.lock")) - }, silent = T) + try(dkill(conn@server, dir = conn@dir), silent = T) } +} #' @export #' @importFrom ps ps_is_running @@ -163,7 +162,7 @@ setMethod("dbIsValid", "DoltLocalConnection", function(dbObj, ...) { class(try(dbGetQuery(dbObj, "SELECT 1"), silent = TRUE)) != "try-error" && ps_is_running(dbObj@server) if (!valid && inherits(dbObj@server, "ps_handle")) - try(dkill(dbObj@server), silent = TRUE) + try(dkill(dbObj@server, dbObj@dir), silent = TRUE) valid }) diff --git a/R/server.R b/R/server.R index a7eb4da..0ce0e1b 100644 --- a/R/server.R +++ b/R/server.R @@ -162,7 +162,7 @@ dolt_server_find <- function(dir = NULL, port = NULL, doltr_only = FALSE) { dolt_server_kill <- function(dir = NULL, port = NULL, doltr_only = FALSE, verbose = TRUE) { dp <- dolt_server_find(dir, port, doltr_only) - lapply(dp$ps_handle, dkill) + lapply(dp$ps_handle, dkill, dir = dir) if (verbose) message(nrow(dp), " processes killed") # dolt now uses a lock file which needs to be cleaned up after process killed @@ -171,11 +171,21 @@ dolt_server_kill <- function(dir = NULL, port = NULL, doltr_only = FALSE, verbos } #' @importFrom ps signals ps_terminate ps_kill -dkill <- function(p = ps_handle) { +dkill <- function(p = ps_handle, dir = NULL) { + # We should prefer SIGTERM over SIGKILL when possible + # is.null(ps::signals()$SIGTERM)) asks if SIGTERM + # is NOT available. if (is.null(ps::signals()$SIGTERM)) { - ps_terminate(p) - } else { + if(is.null(dir)) { + warning("In dkill(): ps_kill() requires connection dir to clean up sql-server.lock. Process not killed") + return(NULL) + } + # If SIGTERM signal is NOT available kill the + # process and clean up the lock file manually. ps_kill(p) + unlink(paste0(dir, "/.dolt/sql-server.lock")) + } else { + ps_terminate(p) # sql-server.lock is cleaned up automatically } invisible(NULL) From f7a9ad7ea42b81812626ef3225ecbb7308893735 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Tue, 2 Aug 2022 13:01:32 -0700 Subject: [PATCH 24/38] Switch to using ps::cwd() to identify p_handle directory rather that adding an argument to dkill() --- R/dolt-local-connection.R | 4 ++-- R/server.R | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index a4edd7c..d5b9753 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -150,7 +150,7 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { getMethod(dbDisconnect, "DoltConnection")(conn) if (kill_server) { - try(dkill(conn@server, dir = conn@dir), silent = T) + try(dkill(conn@server), silent = T) } } @@ -162,7 +162,7 @@ setMethod("dbIsValid", "DoltLocalConnection", function(dbObj, ...) { class(try(dbGetQuery(dbObj, "SELECT 1"), silent = TRUE)) != "try-error" && ps_is_running(dbObj@server) if (!valid && inherits(dbObj@server, "ps_handle")) - try(dkill(dbObj@server, dbObj@dir), silent = TRUE) + try(dkill(dbObj@server), silent = TRUE) valid }) diff --git a/R/server.R b/R/server.R index 0ce0e1b..e9bad52 100644 --- a/R/server.R +++ b/R/server.R @@ -162,7 +162,7 @@ dolt_server_find <- function(dir = NULL, port = NULL, doltr_only = FALSE) { dolt_server_kill <- function(dir = NULL, port = NULL, doltr_only = FALSE, verbose = TRUE) { dp <- dolt_server_find(dir, port, doltr_only) - lapply(dp$ps_handle, dkill, dir = dir) + lapply(dp$ps_handle, dkill) if (verbose) message(nrow(dp), " processes killed") # dolt now uses a lock file which needs to be cleaned up after process killed @@ -171,19 +171,15 @@ dolt_server_kill <- function(dir = NULL, port = NULL, doltr_only = FALSE, verbos } #' @importFrom ps signals ps_terminate ps_kill -dkill <- function(p = ps_handle, dir = NULL) { +dkill <- function(p = ps_handle) { # We should prefer SIGTERM over SIGKILL when possible # is.null(ps::signals()$SIGTERM)) asks if SIGTERM # is NOT available. if (is.null(ps::signals()$SIGTERM)) { - if(is.null(dir)) { - warning("In dkill(): ps_kill() requires connection dir to clean up sql-server.lock. Process not killed") - return(NULL) - } # If SIGTERM signal is NOT available kill the # process and clean up the lock file manually. ps_kill(p) - unlink(paste0(dir, "/.dolt/sql-server.lock")) + unlink(paste0(ps::cwd(p), "/.dolt/sql-server.lock")) } else { ps_terminate(p) # sql-server.lock is cleaned up automatically } From 086f3ab42effb02f78b1fc421b2ebb1159959a34 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Tue, 2 Aug 2022 13:20:24 -0700 Subject: [PATCH 25/38] Fix missing paranthetical and change ps::cwd to ps::ps_cwd --- R/dolt-local-connection.R | 2 +- R/server.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index d5b9753..03e820c 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -152,7 +152,7 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { if (kill_server) { try(dkill(conn@server), silent = T) } -} +}) #' @export #' @importFrom ps ps_is_running diff --git a/R/server.R b/R/server.R index e9bad52..95c8ebb 100644 --- a/R/server.R +++ b/R/server.R @@ -179,7 +179,7 @@ dkill <- function(p = ps_handle) { # If SIGTERM signal is NOT available kill the # process and clean up the lock file manually. ps_kill(p) - unlink(paste0(ps::cwd(p), "/.dolt/sql-server.lock")) + unlink(paste0(ps::ps_cwd(p), "/.dolt/sql-server.lock")) } else { ps_terminate(p) # sql-server.lock is cleaned up automatically } From a07571e7561bb00bcd4a211f9bbe2672f24f8c06 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Thu, 4 Aug 2022 16:13:15 -0700 Subject: [PATCH 26/38] Upate dbReadTable to stop adding space between as of hash and surrounding single quotes --- R/read-table.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/read-table.R b/R/read-table.R index 789974e..b7ccfa4 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -57,7 +57,7 @@ setMethod("dbReadTable", c("DoltConnection", "character"), query_as_of <- function(query, as_of) { as_of <- tryCatch( paste0("TIMESTAMP('", as.character(as.POSIXct(as_of)), "')"), - error = function(e) paste("'", as_of, "'") + error = function(e) paste0("'", as_of, "'") ) query <- paste0(query, " AS OF ", as_of) query From 036577cac3019384884f1a0cc03e62475a029452 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Fri, 5 Aug 2022 09:03:43 -0700 Subject: [PATCH 27/38] Add hash_qualified implementation of 'as_of' argument to dbReadTable for views --- R/dolt-local-connection.R | 4 ++-- R/read-table.R | 23 +++++++++++++++++++---- R/server.R | 3 ++- R/utils.R | 6 ++++++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/R/dolt-local-connection.R b/R/dolt-local-connection.R index 03e820c..9176566 100644 --- a/R/dolt-local-connection.R +++ b/R/dolt-local-connection.R @@ -129,11 +129,11 @@ setMethod("dbDisconnect", "DoltLocalConnection", function(conn, ...) { if (dbIsValid(conn) && ps_is_running(conn@server)) { # On disconnection, kill the server only if it was started by doltr and no - # no other processes connect to it + # no other processes connect to it. is_doltr_server <- isTRUE(ps_environ(conn@server)["R_DOLT"] == "1") procs <- ps() - procs <- procs[procs$status == "running" & procs$pid != ps_pid(ps_handle()),] + procs <- procs[(procs$status == "running" | procs$status == "sleeping") & procs$pid != ps_pid(ps_handle()),] procs <- procs[vapply(procs$ps_handle, function(x) { conns <- try(ps_connections(x), silent = TRUE) out <- !inherits(conns, "try-error") && nrow(conns) && conn@port %in% conns$lport diff --git a/R/read-table.R b/R/read-table.R index b7ccfa4..2d5e985 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -40,8 +40,17 @@ setMethod("dbReadTable", c("DoltConnection", "character"), } name <- dbQuoteIdentifier(conn, name) + + if (!is.null(as_of)) { + table_type <- get_table_type(conn, name) + if(table_type == "VIEW") { + name <- query_hash_qualified(name, as_of) + } else if (table_type == "BASE TABLE") { + name <- query_as_of(name, as_of) + } + } + query <- paste("SELECT * FROM ", name) - if (!is.null(as_of)) query <- query_as_of(query, as_of) out <- dbGetQuery(conn, query, row.names = row.names) @@ -54,13 +63,19 @@ setMethod("dbReadTable", c("DoltConnection", "character"), } ) -query_as_of <- function(query, as_of) { +query_as_of <- function(name, as_of) { as_of <- tryCatch( paste0("TIMESTAMP('", as.character(as.POSIXct(as_of)), "')"), error = function(e) paste0("'", as_of, "'") ) - query <- paste0(query, " AS OF ", as_of) - query + name <- paste0(name, " AS OF ", as_of) + name +} + +query_hash_qualified <- function(conn, name, as_of) { + dbname <- dbGetQuery(conn, "select DATABASE()")[[1]] + name <- paste0("`", dbname, "/", as_of, "`.", name) + name } #' @export diff --git a/R/server.R b/R/server.R index 95c8ebb..a47e5e3 100644 --- a/R/server.R +++ b/R/server.R @@ -181,7 +181,8 @@ dkill <- function(p = ps_handle) { ps_kill(p) unlink(paste0(ps::ps_cwd(p), "/.dolt/sql-server.lock")) } else { - ps_terminate(p) # sql-server.lock is cleaned up automatically + ps_terminate(p) # sql-server.lock should be cleaned up automatically. Just in case though... + unlink(paste0(ps::ps_cwd(p), "/.dolt/sql-server.lock")) } invisible(NULL) diff --git a/R/utils.R b/R/utils.R index 5b8109e..8dcc229 100644 --- a/R/utils.R +++ b/R/utils.R @@ -126,3 +126,9 @@ compact <- function(x) { is_empty <- vapply(x, function(x) length(x) == 0, logical(1)) x[!is_empty] } + +get_table_type <- function(conn = NULL, name = NULL) { + if(is.null(conn) | is.null(name)) return(NULL) + out <- RMariaDB::dbGetQuery(conn, "select * from information_schema.tables") + out |> filter(TABLE_NAME == name) |> pull(TABLE_TYPE) +} From dca09453c17c1e750d8ae5a36c1a006d39de8b0e Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Fri, 5 Aug 2022 14:53:16 -0700 Subject: [PATCH 28/38] Update dbListTable to be as_if friendly and add in dbGetTableType --- R/read-table.R | 30 ++++++++++++++++++------------ R/utils.R | 6 ------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index 2d5e985..7eb0c14 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -42,11 +42,14 @@ setMethod("dbReadTable", c("DoltConnection", "character"), name <- dbQuoteIdentifier(conn, name) if (!is.null(as_of)) { - table_type <- get_table_type(conn, name) - if(table_type == "VIEW") { - name <- query_hash_qualified(name, as_of) - } else if (table_type == "BASE TABLE") { - name <- query_as_of(name, as_of) + table_type <- dbListTableType(conn, name, as_of) + if(!length(table_type)) warning("table does not exist at as_of commit") + if(length(table_type) > 0) { + if(table_type == "VIEW") { + name <- query_hash_qualified(name, as_of) + } else if (table_type == "BASE TABLE") { + name <- query_as_of(name, as_of) + } } } @@ -81,15 +84,18 @@ query_hash_qualified <- function(conn, name, as_of) { #' @export #' @rdname dolt-read setMethod("dbListTables", "DoltConnection", function(conn, as_of = NULL, ...) { - # DATABASE(): https://stackoverflow.com/a/8096574/946850 - query <- paste0("SELECT table_name FROM INFORMATION_SCHEMA.tables\n", - "WHERE table_schema = DATABASE()") - if (!is.null(as_of)) query <- query_as_of(query, as_of) - - dbGetQuery(conn, query)[[1]] - + query <- 'show full tables' + if(!is.null(as_of)) query <- paste0(query, " as of '", as_of, "'") + out <- RMariaDB::dbGetQuery(conn, query) + out[[1]] }) +dbGetTableType <- function(conn, name, as_of = NULL) { + query <- 'show full tables' + if(!is.null(as_of)) query <- paste0(query, " as of '", as_of, "'") + out <- RMariaDB::dbGetQuery(conn, query) + out[out[,1] == name, 2] +} #' @export #' @inheritParams DBI::dbListObjects diff --git a/R/utils.R b/R/utils.R index 8dcc229..5b8109e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -126,9 +126,3 @@ compact <- function(x) { is_empty <- vapply(x, function(x) length(x) == 0, logical(1)) x[!is_empty] } - -get_table_type <- function(conn = NULL, name = NULL) { - if(is.null(conn) | is.null(name)) return(NULL) - out <- RMariaDB::dbGetQuery(conn, "select * from information_schema.tables") - out |> filter(TABLE_NAME == name) |> pull(TABLE_TYPE) -} From 02709aec5d731d42971aac303d14327c7bfbf965 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 9 Jan 2023 09:08:37 -0800 Subject: [PATCH 29/38] Change SELECT syntax on dolt procedures to new CALL format --- R/dolt-nav.R | 2 +- R/dolt-remote.R | 6 +++--- R/dolt-stage-commit.R | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/dolt-nav.R b/R/dolt-nav.R index d8e5228..667100e 100644 --- a/R/dolt-nav.R +++ b/R/dolt-nav.R @@ -17,7 +17,7 @@ dolt_checkout <- function(branch, b = FALSE, start_point = NULL, branch = sql_quote(branch, "'") if (b) branch <- paste0(sql_quote("-b", "'"), ", ", branch) if (!is.null(start_point)) branch <- paste0(branch, ", ", start_point) - query <- paste0("select dolt_checkout(", branch, ")") + query <- paste0("call dolt_checkout(", branch, ")") dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } diff --git a/R/dolt-remote.R b/R/dolt-remote.R index 8685c4c..f8c00ac 100644 --- a/R/dolt-remote.R +++ b/R/dolt-remote.R @@ -23,7 +23,7 @@ dolt_push <- function(remote = NULL, remote_branch = NULL, ref = NULL, if (!is.null (ref)) args <- c(args, sql_quote(ref, "'")) if (set_upstream) args <- c("'--set-upstream' ", args) if (force) args <- c(args, "'--force'") - query <- paste0("select dolt_push(", paste0(args, collapse = ", "), ")") + query <- paste0("call dolt_push(", paste0(args, collapse = ", "), ")") dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } @@ -39,7 +39,7 @@ dolt_pull <- function(remote = NULL, squash = FALSE, conn = dolt(), args <- "" if (!is.null(remote)) args <- c(args, sql_quote(remote, "'")) if (squash) args <- c(args, "'--squash'") - query <- paste0("select dolt_pull(", paste0(args, collapse = ", "), ")") + query <- paste0("call dolt_pull(", paste0(args, collapse = ", "), ")") dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } @@ -54,7 +54,7 @@ dolt_fetch <- function(remote = NULL, ref = FALSE, force = FALSE, if (!is.null(remote)) args <- c(args, sql_quote(remote, "'")) if (!is.null(ref)) args <- c(args, sql_quote(remote, "'")) if (force) args <- c(args, "'--force'") - query <- paste0("select dolt_fetch(", paste0(args, collapse = ", "), ")") + query <- paste0("call dolt_fetch(", paste0(args, collapse = ", "), ")") dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } diff --git a/R/dolt-stage-commit.R b/R/dolt-stage-commit.R index 15cf58d..a44412c 100644 --- a/R/dolt-stage-commit.R +++ b/R/dolt-stage-commit.R @@ -9,7 +9,7 @@ dolt_add <- function(tables = NULL, conn = dolt(), collect = NULL, show_sql = NU tables <- "'--all'" else tables <- paste0("'", tables, "'", collapse = ", ") - query <- paste0("select dolt_add(", tables, ")"); + query <- paste0("call dolt_add(", tables, ")"); dolt_query(query, conn, collect, show_sql) invisible(dolt_status()) } @@ -44,7 +44,7 @@ dolt_commit <- function(all = TRUE, message = NULL, author = NULL, date = NULL, if (!is.null(author)) args <- c(args, "'--author'", paste0("'", author, "'")) if (!is.null(date)) args <- c(args, "'--date'", paste0("'", date, "'")) if (allow_empty) args <- c(args, "'--allow-empty'") - query <- paste0("select dolt_commit(", paste0(args, collapse = ", "), ")"); + query <- paste0("call dolt_commit(", paste0(args, collapse = ", "), ")"); dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } From e89589ef9a0251d723c3e96b3ee4cf585b80db4b Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 9 Jan 2023 09:33:28 -0800 Subject: [PATCH 30/38] dolt_query uses dplyr's tbl which cannot handle dolt's new CALL syntax. I added a new argument to dolt_query named `execute` which will use `RMariaDB::dbExecute` instead of `tbl` when set to true. --- R/dolt-nav.R | 4 ++-- R/dolt-stage-commit.R | 2 +- R/query.R | 9 +++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/R/dolt-nav.R b/R/dolt-nav.R index 667100e..bd4d435 100644 --- a/R/dolt-nav.R +++ b/R/dolt-nav.R @@ -17,8 +17,8 @@ dolt_checkout <- function(branch, b = FALSE, start_point = NULL, branch = sql_quote(branch, "'") if (b) branch <- paste0(sql_quote("-b", "'"), ", ", branch) if (!is.null(start_point)) branch <- paste0(branch, ", ", start_point) - query <- paste0("call dolt_checkout(", branch, ")") - dolt_query(query, conn, collect, show_sql) + query <- paste0("CALL dolt_checkout(", branch, ")") + dolt_query(query, conn, collect, show_sql, execute = T) invisible(dolt_state()) } diff --git a/R/dolt-stage-commit.R b/R/dolt-stage-commit.R index a44412c..21ff372 100644 --- a/R/dolt-stage-commit.R +++ b/R/dolt-stage-commit.R @@ -62,7 +62,7 @@ dolt_reset <- function(hard = FALSE, tables = NULL, conn = dolt(), args <- c() if (!is.null(tables)) args <- paste0(sql_quote(tables, "'"), collapse = ", ") if (hard) args <- c(sql_quote("--hard", "'"), args) - query <- paste0("select dolt_reset(", paste(args, collapse = ", ", ")")) + query <- paste0("call dolt_reset(", paste(args, collapse = ", ", ")")) dolt_query(query, conn, collect, show_sql) invisible(dolt_state()) } diff --git a/R/query.R b/R/query.R index e39a3ef..ae25f3d 100644 --- a/R/query.R +++ b/R/query.R @@ -1,10 +1,15 @@ #' @importFrom dplyr tbl collect sql dolt_query <- function(query, conn = dolt(), collect = Sys.getboolenv("DOLT_COLLECT", TRUE), - show_sql = Sys.getboolenv("DOLT_VERBOSE", FALSE)) { + show_sql = Sys.getboolenv("DOLT_VERBOSE", FALSE), + execute = F) { query <- sql(query) if (show_sql) message(query) - result <- tbl(conn, query) + if(!execute) { + result <- tbl(conn, query) + } else { + result <- RMariaDB::dbExecute(conn, query) + } if (collect) result <- collect(result) result } From 6cccad864e7b25aad49ac6007701cb42efc33d98 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 9 Jan 2023 09:35:55 -0800 Subject: [PATCH 31/38] Updated other instances of CALL to use new dolt_query execute = T syntax --- R/dolt-remote.R | 2 +- R/dolt-stage-commit.R | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/dolt-remote.R b/R/dolt-remote.R index f8c00ac..722ced2 100644 --- a/R/dolt-remote.R +++ b/R/dolt-remote.R @@ -24,7 +24,7 @@ dolt_push <- function(remote = NULL, remote_branch = NULL, ref = NULL, if (set_upstream) args <- c("'--set-upstream' ", args) if (force) args <- c(args, "'--force'") query <- paste0("call dolt_push(", paste0(args, collapse = ", "), ")") - dolt_query(query, conn, collect, show_sql) + dolt_query(query, conn, collect, show_sql, execute = T) invisible(dolt_state()) } diff --git a/R/dolt-stage-commit.R b/R/dolt-stage-commit.R index 21ff372..ec97705 100644 --- a/R/dolt-stage-commit.R +++ b/R/dolt-stage-commit.R @@ -10,7 +10,7 @@ dolt_add <- function(tables = NULL, conn = dolt(), collect = NULL, show_sql = NU else tables <- paste0("'", tables, "'", collapse = ", ") query <- paste0("call dolt_add(", tables, ")"); - dolt_query(query, conn, collect, show_sql) + dolt_query(query, conn, collect, show_sql, execute = T) invisible(dolt_status()) } @@ -45,7 +45,7 @@ dolt_commit <- function(all = TRUE, message = NULL, author = NULL, date = NULL, if (!is.null(date)) args <- c(args, "'--date'", paste0("'", date, "'")) if (allow_empty) args <- c(args, "'--allow-empty'") query <- paste0("call dolt_commit(", paste0(args, collapse = ", "), ")"); - dolt_query(query, conn, collect, show_sql) + dolt_query(query, conn, collect, show_sql, execute = T) invisible(dolt_state()) } @@ -63,6 +63,6 @@ dolt_reset <- function(hard = FALSE, tables = NULL, conn = dolt(), if (!is.null(tables)) args <- paste0(sql_quote(tables, "'"), collapse = ", ") if (hard) args <- c(sql_quote("--hard", "'"), args) query <- paste0("call dolt_reset(", paste(args, collapse = ", ", ")")) - dolt_query(query, conn, collect, show_sql) + dolt_query(query, conn, collect, show_sql, execute = T) invisible(dolt_state()) } From 3a85762961db667afa04b0fb958dc89e938b7fbd Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 9 Jan 2023 09:42:20 -0800 Subject: [PATCH 32/38] Collect doesn't apply to dbExecute results. Removed that option when execute = T in dolt_query --- R/query.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/query.R b/R/query.R index ae25f3d..6f43ed0 100644 --- a/R/query.R +++ b/R/query.R @@ -7,10 +7,10 @@ dolt_query <- function(query, conn = dolt(), if (show_sql) message(query) if(!execute) { result <- tbl(conn, query) + if (collect) result <- collect(result) } else { result <- RMariaDB::dbExecute(conn, query) } - if (collect) result <- collect(result) result } From 03836df72470c8edf67643d25716ed5703ec1250 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 9 Jan 2023 14:49:56 -0800 Subject: [PATCH 33/38] Split out dolt_call --- R/dolt-nav.R | 4 ++-- R/dolt-remote.R | 6 +++--- R/dolt-stage-commit.R | 10 ++++++---- R/query.R | 22 +++++++++++++--------- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/R/dolt-nav.R b/R/dolt-nav.R index bd4d435..e8a8f31 100644 --- a/R/dolt-nav.R +++ b/R/dolt-nav.R @@ -18,8 +18,8 @@ dolt_checkout <- function(branch, b = FALSE, start_point = NULL, if (b) branch <- paste0(sql_quote("-b", "'"), ", ", branch) if (!is.null(start_point)) branch <- paste0(branch, ", ", start_point) query <- paste0("CALL dolt_checkout(", branch, ")") - dolt_query(query, conn, collect, show_sql, execute = T) - invisible(dolt_state()) + dolt_call(query, conn, show_sql) + dolt_state() # I don't think this needs to be invisible. } #' @export diff --git a/R/dolt-remote.R b/R/dolt-remote.R index 722ced2..8721702 100644 --- a/R/dolt-remote.R +++ b/R/dolt-remote.R @@ -24,7 +24,7 @@ dolt_push <- function(remote = NULL, remote_branch = NULL, ref = NULL, if (set_upstream) args <- c("'--set-upstream' ", args) if (force) args <- c(args, "'--force'") query <- paste0("call dolt_push(", paste0(args, collapse = ", "), ")") - dolt_query(query, conn, collect, show_sql, execute = T) + dolt_call(query, conn, show_sql) invisible(dolt_state()) } @@ -40,7 +40,7 @@ dolt_pull <- function(remote = NULL, squash = FALSE, conn = dolt(), if (!is.null(remote)) args <- c(args, sql_quote(remote, "'")) if (squash) args <- c(args, "'--squash'") query <- paste0("call dolt_pull(", paste0(args, collapse = ", "), ")") - dolt_query(query, conn, collect, show_sql) + dolt_call(query, conn, show_sql) invisible(dolt_state()) } @@ -55,7 +55,7 @@ dolt_fetch <- function(remote = NULL, ref = FALSE, force = FALSE, if (!is.null(ref)) args <- c(args, sql_quote(remote, "'")) if (force) args <- c(args, "'--force'") query <- paste0("call dolt_fetch(", paste0(args, collapse = ", "), ")") - dolt_query(query, conn, collect, show_sql) + dolt_call(query, conn, show_sql) invisible(dolt_state()) } diff --git a/R/dolt-stage-commit.R b/R/dolt-stage-commit.R index ec97705..db4f524 100644 --- a/R/dolt-stage-commit.R +++ b/R/dolt-stage-commit.R @@ -10,7 +10,7 @@ dolt_add <- function(tables = NULL, conn = dolt(), collect = NULL, show_sql = NU else tables <- paste0("'", tables, "'", collapse = ", ") query <- paste0("call dolt_add(", tables, ")"); - dolt_query(query, conn, collect, show_sql, execute = T) + dolt_call(query, conn, show_sql) invisible(dolt_status()) } @@ -45,8 +45,10 @@ dolt_commit <- function(all = TRUE, message = NULL, author = NULL, date = NULL, if (!is.null(date)) args <- c(args, "'--date'", paste0("'", date, "'")) if (allow_empty) args <- c(args, "'--allow-empty'") query <- paste0("call dolt_commit(", paste0(args, collapse = ", "), ")"); - dolt_query(query, conn, collect, show_sql, execute = T) - invisible(dolt_state()) + dolt_call(query, conn, show_sql) + state <- dolt_state() + message(paste0("head commit: ", state$head)) + invisible(state) } #' @param hard Reset working and staged tables? If FALSE (default), a "soft" @@ -63,6 +65,6 @@ dolt_reset <- function(hard = FALSE, tables = NULL, conn = dolt(), if (!is.null(tables)) args <- paste0(sql_quote(tables, "'"), collapse = ", ") if (hard) args <- c(sql_quote("--hard", "'"), args) query <- paste0("call dolt_reset(", paste(args, collapse = ", ", ")")) - dolt_query(query, conn, collect, show_sql, execute = T) + dolt_call(query, conn, show_sql) invisible(dolt_state()) } diff --git a/R/query.R b/R/query.R index 6f43ed0..a29d309 100644 --- a/R/query.R +++ b/R/query.R @@ -1,20 +1,14 @@ #' @importFrom dplyr tbl collect sql dolt_query <- function(query, conn = dolt(), collect = Sys.getboolenv("DOLT_COLLECT", TRUE), - show_sql = Sys.getboolenv("DOLT_VERBOSE", FALSE), - execute = F) { + show_sql = Sys.getboolenv("DOLT_VERBOSE", FALSE)) { query <- sql(query) if (show_sql) message(query) - if(!execute) { - result <- tbl(conn, query) - if (collect) result <- collect(result) - } else { - result <- RMariaDB::dbExecute(conn, query) - } + result <- tbl(conn, query) + if (collect) result <- collect(result) result } - .collect <- function(collect) { if (is.null(collect)) return(Sys.getboolenv("DOLT_COLLECT", TRUE)) @@ -28,3 +22,13 @@ dolt_query <- function(query, conn = dolt(), else return(show_sql) } + +dolt_call <- function(query, conn = dolt(), + show_sql = Sys.getboolenv("DOLT_VERBOSE", FALSE)) { + + query <- sql(query) + if (show_sql) message(query) + result <- RMariaDB::dbExecute(conn, query) + result +} + From 11eb989b7f1ea3c912ccda972481aa1f03abacae Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Wed, 1 Mar 2023 14:29:07 -0800 Subject: [PATCH 34/38] Fix as_of for views --- R/read-table.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index 7eb0c14..562a6cd 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -42,13 +42,13 @@ setMethod("dbReadTable", c("DoltConnection", "character"), name <- dbQuoteIdentifier(conn, name) if (!is.null(as_of)) { - table_type <- dbListTableType(conn, name, as_of) + table_type <- dbGetTableType(conn, name, as_of) if(!length(table_type)) warning("table does not exist at as_of commit") if(length(table_type) > 0) { if(table_type == "VIEW") { - name <- query_hash_qualified(name, as_of) + name <- query_hash_qualified(dolt(), name, as_of) } else if (table_type == "BASE TABLE") { - name <- query_as_of(name, as_of) + name <- query_as_of(dolt(), name, as_of) } } } From 8de34a5f2a414e60e4c2124bbe44ee8e4669a34b Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Wed, 1 Mar 2023 17:21:37 -0800 Subject: [PATCH 35/38] Set the behavior of `as_of` in dbReadTable() to generate the view using the view definition and data as they were at the specified commit. --- R/read-table.R | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index 562a6cd..f804284 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -39,23 +39,20 @@ setMethod("dbReadTable", c("DoltConnection", "character"), stopc("`check.names` must be a logical scalar") } - name <- dbQuoteIdentifier(conn, name) - if (!is.null(as_of)) { table_type <- dbGetTableType(conn, name, as_of) + print(paste0(name, as_of, "table_type:", table_type)) if(!length(table_type)) warning("table does not exist at as_of commit") - if(length(table_type) > 0) { - if(table_type == "VIEW") { - name <- query_hash_qualified(dolt(), name, as_of) - } else if (table_type == "BASE TABLE") { - name <- query_as_of(dolt(), name, as_of) - } - } + name <- query_hash_qualified(conn, name, as_of) + } else { + name <- dbQuoteIdentifier(conn, name) } query <- paste("SELECT * FROM ", name) + print(query) - out <- dbGetQuery(conn, query, + out <- dbGetQuery(conn, + query, row.names = row.names) if (check.names) { From 52917681f428bd3a0ac5480cf8421b773c7e08b4 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Wed, 1 Mar 2023 20:25:22 -0800 Subject: [PATCH 36/38] Remove diagnostic print statements --- R/read-table.R | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index f804284..5b24972 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -28,7 +28,13 @@ NULL #' @export #' @rdname dolt-read setMethod("dbReadTable", c("DoltConnection", "character"), - function(conn, name, as_of = NULL, ..., row.names = FALSE, check.names = TRUE) { + function(conn, name, + as_of = NULL, + ..., + row.names = FALSE, + check.names = TRUE, + show_sql = F) { + row.names <- compatRowNames(row.names) if ((!is.logical(row.names) && !is.character(row.names)) || length(row.names) != 1L) { @@ -41,7 +47,6 @@ setMethod("dbReadTable", c("DoltConnection", "character"), if (!is.null(as_of)) { table_type <- dbGetTableType(conn, name, as_of) - print(paste0(name, as_of, "table_type:", table_type)) if(!length(table_type)) warning("table does not exist at as_of commit") name <- query_hash_qualified(conn, name, as_of) } else { @@ -49,7 +54,7 @@ setMethod("dbReadTable", c("DoltConnection", "character"), } query <- paste("SELECT * FROM ", name) - print(query) + if(show_sql) print(query) out <- dbGetQuery(conn, query, From 8103e0bf8d48220c5b52b5179fc49cbcaf2cec57 Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Wed, 1 Mar 2023 21:28:17 -0800 Subject: [PATCH 37/38] Previous method didn't actually work on testing and failed silently. This works. --- R/read-table.R | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index 5b24972..8473275 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -45,18 +45,20 @@ setMethod("dbReadTable", c("DoltConnection", "character"), stopc("`check.names` must be a logical scalar") } - if (!is.null(as_of)) { - table_type <- dbGetTableType(conn, name, as_of) - if(!length(table_type)) warning("table does not exist at as_of commit") - name <- query_hash_qualified(conn, name, as_of) - } else { - name <- dbQuoteIdentifier(conn, name) - } + # if (!is.null(as_of)) { + # table_type <- dbGetTableType(conn, name, as_of) + # if(!length(table_type)) warning("table does not exist at as_of commit") + # name <- query_hash_qualified(conn, name, as_of) + # } else { + # name <- dbQuoteIdentifier(conn, name) + # } + + query <- paste("SELECT * FROM", name) - query <- paste("SELECT * FROM ", name) + if(!is.null(as_of)) query <- paste(query, "AS OF", DBI::dbQuoteString(dolt(), as_of)) if(show_sql) print(query) - out <- dbGetQuery(conn, + out <- DBI::dbGetQuery(conn, query, row.names = row.names) From ffb5bc68003e83ebdb9f352654bab2515ca6bf3a Mon Sep 17 00:00:00 2001 From: Nathan Layman Date: Mon, 12 Jun 2023 11:46:18 -0700 Subject: [PATCH 38/38] Uncomment dbGetTableType --- R/read-table.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/read-table.R b/R/read-table.R index 8473275..92de575 100644 --- a/R/read-table.R +++ b/R/read-table.R @@ -45,13 +45,13 @@ setMethod("dbReadTable", c("DoltConnection", "character"), stopc("`check.names` must be a logical scalar") } - # if (!is.null(as_of)) { - # table_type <- dbGetTableType(conn, name, as_of) - # if(!length(table_type)) warning("table does not exist at as_of commit") - # name <- query_hash_qualified(conn, name, as_of) - # } else { - # name <- dbQuoteIdentifier(conn, name) - # } + if (!is.null(as_of)) { + table_type <- dbGetTableType(conn, name, as_of) + if(!length(table_type)) warning("table does not exist at as_of commit") + name <- query_hash_qualified(conn, name, as_of) + } else { + name <- dbQuoteIdentifier(conn, name) + } query <- paste("SELECT * FROM", name)