You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/07/22 15:20:13 UTC

[GitHub] [arrow] jonkeane opened a new pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

jonkeane opened a new pull request #10780:
URL: https://github.com/apache/arrow/pull/10780


   A proposed interface for using DuckDB + Arrow together.
   
   I've added two methods:
     * The proposed `summarise(..., .engine = "duckdb")` method which is (probably) the method that people want to use
     * A lower-level method of specifying exactly when the transfer takes place. I've called this `alchemize_*` for now, though we might consider wedging it into `collect()` or `compute()` (or something like `collect_to_duckdb()` to be super explicit[1]).
     * I've made a proof-of-concept that the `alchemize_*` can also work with Python β€” this is basically a renaming/wrapping of `r_to_py` / `py_to_r`. If we do peruse exposing `alchemize_*` or the like, I will fill out the rest of these (we should keep both around, though r_to_py isn't currently documented so probably isn't getting much use).
     
     [1] I've tried both a more magical `alchemize(x, to = c("arrow", "duckdb", "python))` the changes behavior / output based on the `to` argument, which we can go back to if we want that simplicity, but I found it harder to reason about what I was getting out. Where as with `alchemize_to_duckdb()` the function says exactly what's going on.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675128474



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A tbl of the new table in DuckDB
+#'
+#' @keywords internal
+#' @name to_duckdb
+NULL
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+.alchemize_to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb <- function(src, con, table_name, auto_disconnect) UseMethod("to_duckdb", src)
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.Dataset <- .alchemize_to_duckdb
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.Table <- .alchemize_to_duckdb
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.arrow_dplyr_query <- .alchemize_to_duckdb
+
+summarise_duck <- function(.data, ...) {
+  # TODO: pass a connection?

Review comment:
       I don't think so--if the user wants more control over the duckdb connection, they can call `to_duckdb()` explicitly rather than implicitly here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675127116



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A tbl of the new table in DuckDB
+#'
+#' @keywords internal
+#' @name to_duckdb
+NULL
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+.alchemize_to_duckdb <- function(

Review comment:
       I'd just do this, attach the rd doclets directly here, and drop the S3 method business below, I don't think it gives you anything useful
   
   ```suggestion
   to_duckdb <- function(
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675737997



##########
File path: r/_pkgdown.yml
##########
@@ -164,6 +164,9 @@ reference:
       - match_arrow
       - value_counts
       - list_compute_functions
+  - title: Connections to other systems
+    contents:
+      - to_duckdb

Review comment:
       We should, though I'm going to kick that can down the road with https://issues.apache.org/jira/browse/ARROW-13442 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675084799



##########
File path: r/R/alchemize.R
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Transform a data structure from one engine to another

Review comment:
       πŸ€” I'm working on this right now and I'm torn. I like the idea of having one function / family of them that clearly do this kind of transformation (right now it's relatively limited, but we've got 2 already!).
   
   `tbl` works pretty well, though right now I'm trying to figure out what the right answer is for `tbl(..., .engine = "arrow")`. In an ideal world(?) duckdb would also expose a C-interface for us to get Arrow data out of, such that one could pull data back in to Arrow without needing to route via R which this would be exactly that (though AFAICT that doesn't currently exist).
   
   I have right now (with the comment in the middle not totally sure what to do)
   ```
   tbl.arrow_dplyr_query <- function(src, ..., .engine = c("arrow", "duckdb")) {
     engine <- match.arg(.engine)
   
     if (engine == "arrow") {
       # What should we do here? Possibilities:
       # stop("not yet implemented")?
       # return(src) # no-op + maybe a warning that we did nothing
       # return(arrow_dplyr_query(src)) # this is the most tbl like thing for an Arrow object
     } else if (engine == "duckdb") {
       .alchemize_to_duckdb(src = src, ...)
     }
   }
   ```
   
   So maybe it is worth waiting until some of those other features exist (or the next hand-over integration comes up) before we come up with a grand scheme that unifies these kinds of hand offs. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675131832



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")
+
+# write one table to the connection so it is kept open
+DBI::dbWriteTable(con, "mtcars", mtcars)
+
+test_that("to_duckdb", {
+  ds <- InMemoryDataset$create(example_data)
+
+  expect_identical(
+    ds %>%
+      to_duckdb() %>%
+      collect() %>%
+      # factors don't roundtrip
+      select(!fct),
+    select(example_data, !fct)
+  )
+
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      to_duckdb() %>%
+      group_by(lgl) %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+
+  # can group_by before the to_duckdb
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      to_duckdb() %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("summarise(..., .engine)", {
+  ds <- InMemoryDataset$create(example_data)
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      summarise(
+        mean_int = mean(int, na.rm = TRUE),
+        mean_dbl = mean(dbl, na.rm = TRUE),
+        .engine = "duckdb"
+      ) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("Joining, auto-cleanup", {
+  ds <- InMemoryDataset$create(example_data)
+
+  # we always want to test in parallel
+  dbExecute(con, "PRAGMA threads=2")
+
+  table_one <- to_duckdb(ds, con = con)
+  table_one_name <- as.character(table_one$ops$x)

Review comment:
       I also wonder if there's an S3 method or something to add to duckdb so that `tbl(duckdb::duckdb(), ds)` can do the right thing for an arrow dataset




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675075896



##########
File path: r/DESCRIPTION
##########
@@ -40,9 +40,11 @@ Roxygen: list(markdown = TRUE, r6 = FALSE, load = "source")
 RoxygenNote: 7.1.1.9001
 VignetteBuilder: knitr
 Suggests:
+    DBI,
     decor,
     distro,
     dplyr,
+    duckdb (> 0.2.7),

Review comment:
       Do we need to suggest dbplyr too?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675010485



##########
File path: r/R/python.R
##########
@@ -15,6 +15,19 @@
 # specific language governing permissions and limitations
 # under the License.
 
+#' Transfer an Arrow memory structure to Python
+#'
+#' @param x the R-based Arrow object to export to Python
+#' @param ... arguments passed to the methods
+#'
+#' @retuen a reticulate-based Python object
+#'
+#' @keywords internal
+#' @export
+r_to_py <- function(x, ...) {

Review comment:
       Ah, I didn't realize that was coming from reticulate β€” I think that would solve the error though that will be irrelevant if we take out `alchemize_to_python`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675011947



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,91 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# TODO: note that this is copied from dbplyr

Review comment:
       _nods_ we also should include info about this in LICENSE.txt too, or is it changed enough to not also need that?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675127935



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A tbl of the new table in DuckDB
+#'
+#' @keywords internal
+#' @name to_duckdb
+NULL
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+.alchemize_to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb <- function(src, con, table_name, auto_disconnect) UseMethod("to_duckdb", src)
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.Dataset <- .alchemize_to_duckdb
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.Table <- .alchemize_to_duckdb
+
+#' @rdname to_duckdb
+#' @export
+to_duckdb.arrow_dplyr_query <- .alchemize_to_duckdb
+
+summarise_duck <- function(.data, ...) {

Review comment:
       If this is all that summarize_duck does, we can just inline it in summarize.arrow_dplyr_query




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675991803



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,188 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+test_that("to_duckdb", {
+  ds <- InMemoryDataset$create(example_data)
+
+  expect_identical(
+    ds %>%
+      to_duckdb() %>%
+      collect() %>%
+      # factors don't roundtrip
+      select(!fct),
+    select(example_data, !fct)
+  )
+
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      to_duckdb() %>%
+      group_by(lgl) %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+
+  # can group_by before the to_duckdb
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      to_duckdb() %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("summarise(..., .engine)", {
+  ds <- InMemoryDataset$create(example_data)
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      summarise(
+        mean_int = mean(int, na.rm = TRUE),
+        mean_dbl = mean(dbl, na.rm = TRUE),
+        .engine = "duckdb"
+      ) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+# The next set of tests use an already-extant connection to test features of
+# persistence and querying against the table without using the `tbl` itself, so
+# we need to create a connection separate from the ephemeral one that is made
+# with arrow_duck_connection()
+con <- dbConnect(duckdb::duckdb())
+dbExecute(con, "PRAGMA threads=2")
+
+# write one table to the connection so it is kept open
+DBI::dbWriteTable(con, "mtcars", mtcars)
+
+test_that("Joining, auto-cleanup", {
+  ds <- InMemoryDataset$create(example_data)
+
+  table_one_name <- "my_arrow_table_1"
+  table_one <- to_duckdb(ds, con = con, table_name = table_one_name)
+  table_two_name <- "my_arrow_table_2"
+  table_two <- to_duckdb(ds, con = con, table_name = table_two_name)
+
+  res <- dbGetQuery(
+    con,
+    paste0(
+      "SELECT * FROM ", table_one_name,
+      " INNER JOIN ", table_two_name,
+      " ON ", table_one_name, ".int = ", table_two_name, ".int"
+    )
+  )
+  expect_identical(dim(res), c(9L, 14L))
+
+  # clean up cleans up the tables
+  expect_true(all(c(table_one_name, table_two_name) %in% DBI::dbListTables(con)))
+  rm(table_one, table_two)
+  gc()
+  expect_false(any(c(table_one_name, table_two_name) %in% DBI::dbListTables(con)))
+})
+
+test_that("Joining, auto-cleanup disabling", {
+  ds <- InMemoryDataset$create(example_data)
+
+  table_three_name <- "my_arrow_table_3"
+  table_three <- to_duckdb(ds, con = con, table_name = table_three_name, auto_disconnect = FALSE)
+
+  # clean up does *not* clean these tables
+  expect_true(table_three_name %in% DBI::dbListTables(con))
+  rm(table_three)
+  gc()
+  # but because we aren't auto_disconnecting then we still have this table.
+  expect_true(table_three_name %in% DBI::dbListTables(con))
+})
+
+test_that("to_duckdb with a table", {
+  tab <- Table$create(example_data)
+
+  expect_identical(
+    tab %>%
+      to_duckdb() %>%
+      group_by(int > 4) %>%
+      summarise(
+        int_mean = mean(int, na.rm = TRUE),
+        dbl_mean = mean(dbl, na.rm = TRUE)
+      ) %>%
+    collect(),
+    tibble::tibble(
+      "int > 4" = c(FALSE, NA, TRUE),
+      int_mean = c(2, NA, 7.5),
+      dbl_mean = c(2.1, 4.1, 7.3)
+    )
+  )
+})
+
+test_that("to_duckdb passing a connection", {
+  ds <- InMemoryDataset$create(example_data)
+
+  con_separate <- dbConnect(duckdb::duckdb())
+  # we always want to test in parallel
+  dbExecute(con_separate, "PRAGMA threads=2")
+
+  # create a table to join to that we know is in our con_separate
+  new_df <- data.frame(
+    int = 1:10,
+    char = letters[26:17]
+  )
+  DBI::dbWriteTable(con_separate, "separate_join_table", new_df)
+
+  table_four <- ds %>%
+    select(int, lgl, dbl) %>%
+    to_duckdb(con = con_separate)
+  table_four_name <- table_four$ops$x
+
+  result <- DBI::dbGetQuery(
+    con_separate,
+    paste0(
+      "SELECT * FROM ", table_four_name,
+      " INNER JOIN separate_join_table ",
+      "ON separate_join_table.int = ", table_four_name, ".int"
+    )
+  )
+
+  expect_identical(dim(result), c(9L, 5L))
+  expect_identical(result$char, new_df[new_df$int != 4, ]$char)
+
+  dbDisconnect(con_separate, shutdown = TRUE)
+})
+
+dbDisconnect(con, shutdown = TRUE)

Review comment:
       Should we put these dbDisconnects in an `on.exit()` up by their corresponding dbConnects?

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param .data the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(.data,
+                      con = arrow_duck_connection(),
+                      table_name =  unique_arrow_tablename(),
+                      auto_disconnect = TRUE) {
+  .data <- arrow_dplyr_query(.data)
+  duckdb::duckdb_register_arrow(con, table_name, .data)
+
+  tbl <- tbl(con, table_name)
+  groups <- dplyr::groups(.data)
+  if (length(groups)) {
+    tbl <- dplyr::group_by(tbl, groups)
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {

Review comment:
       The null check seems like it should be unnecessary (should we make a PR to DBI?)
   
   ```
   > DBI::dbIsValid(NULL)
   Error in (function (classes, fdef, mtable)  : 
     unable to find an inherited method for function β€˜dbIsValid’ for signature β€˜"NULL"’
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675741443



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")

Review comment:
       Ah, _those_ connections already do have that set. This is actually not really the right place for this connection, since it's only used later in the joining, etc. tests. I've moved this down there so it's closer to where it's used and added a comment explaining why the separate connection is being made + what's going on with it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675138094



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")
+
+# write one table to the connection so it is kept open
+DBI::dbWriteTable(con, "mtcars", mtcars)
+
+test_that("to_duckdb", {
+  ds <- InMemoryDataset$create(example_data)
+
+  expect_identical(
+    ds %>%
+      to_duckdb() %>%
+      collect() %>%
+      # factors don't roundtrip
+      select(!fct),
+    select(example_data, !fct)
+  )
+
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      to_duckdb() %>%
+      group_by(lgl) %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+
+  # can group_by before the to_duckdb
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      to_duckdb() %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("summarise(..., .engine)", {
+  ds <- InMemoryDataset$create(example_data)
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      summarise(
+        mean_int = mean(int, na.rm = TRUE),
+        mean_dbl = mean(dbl, na.rm = TRUE),
+        .engine = "duckdb"
+      ) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("Joining, auto-cleanup", {
+  ds <- InMemoryDataset$create(example_data)
+
+  # we always want to test in parallel
+  dbExecute(con, "PRAGMA threads=2")
+
+  table_one <- to_duckdb(ds, con = con)
+  table_one_name <- as.character(table_one$ops$x)

Review comment:
       That's part of the confusion with `tbl` there, with (most?) sql backends, that's not how `tbl` is intended to work, it's really "just" for selecting a table (see below). We might extend it to do that, but that wouldn't match other backends.
   
   We might want to write a method for `DBI::CreateTable(duckdb::duckdb(), name = "name", value = ds)` (and possibly the older `WriteTable`, though that's supposed to be deprecated). Though that might be confusing if the data is being written in some way. Then again, that's basically `duckdb:: duckdb_register_arrow()` does 
   
   ``` r
   library(duckdb)
   #> Loading required package: DBI
   library(dplyr)
   #> 
   #> Attaching package: 'dplyr'
   #> The following objects are masked from 'package:stats':
   #> 
   #>     filter, lag
   #> The following objects are masked from 'package:base':
   #> 
   #>     intersect, setdiff, setequal, union
   
   con <- DBI::dbConnect(duckdb::duckdb())
   
   tbl(con, mtcars)
   #> Error in UseMethod("as.sql"): no applicable method for 'as.sql' applied to an object of class "data.frame"
   
   
   sqlite_con <- DBI::dbConnect(RSQLite::SQLite())
   
   tbl(sqlite_con, mtcars)
   #> Error in UseMethod("as.sql"): no applicable method for 'as.sql' applied to an object of class "data.frame"
   ```
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#issuecomment-884999100


   https://issues.apache.org/jira/browse/ARROW-12688


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676617698



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       Why not also retain only the columns we need to do aggregations when `.engine == "duckdb"`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676630184



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       I already have something that works, and I think we'll conflict anyway, so I don't think there's any harm keeping it in here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r674976571



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,192 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")
+
+test_that("basic integration", {

Review comment:
       Isn't this testing only functions in `duckdb`, not `arrow`?

##########
File path: r/R/alchemize.R
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Transform a data structure from one engine to another
+#'
+#' The `alchemize_*` family of functions take data in one context (e.g. Arrow
+#' data in an R session, Arrow data in a Python session) and transform it into a
+#' form usable by another context: (e.g. Arrow data in a Python session, a
+#' (virtual) table in a DuckDB session). All of these functions use Arrow's
+#' C-interface and data is not serialized or moved when it is alchemized,
+#' instead it is made available for a subprocess of the new context (e.g. Python
+#' through reticulate or the DuckDB engine).
+#'
+#' The return value is for each function in the family based on what is at the
+#' end of the function name:
+#'
+#' * `alchemize_to_duckdb` - returns a dbplyr-based `tbl` with the Arrow data
+#' registered as a (virtual) table in DuckDB. The `tbl` can be used in dplyr
+#' pipelines, or you can write DuckDB queries using the table name (by default
+#' `"arrow_"` with numbers following it) given in the `tbl`. If you would like
+#' to use a specific, pre-existent connection to DuckDB use the `con` argument
+#' to pass the connection to use. By default, these tables are automatically
+#' cleaned up when the `tbl` is removed from the session (and garbage collection
+#' occurs on that), to disable this, pass `auto_disconnect = FALSE`. *
+#' `alchemize_to_python` - returns a reticulate-based python object. This is the
+#' same as the interface using the `r_to_py` functions.
+#'
+#' @param x the object to alchemize
+#' @param ... arguments passed to other functions
+#'
+#' @return An object with a reference to the the alchemized data
+#'
+#' @keywords internal
+#' @name alchemize
+NULL
+
+#' @rdname alchemize
+#' @export
+alchemize_to_duckdb <- function(x, ...) {
+  UseMethod("alchemize_to_duckdb")
+}
+
+#' @rdname alchemize
+#' @export
+alchemize_to_python <- function(x, ...) {
+  UseMethod("alchemize_to_python")
+}
+
+#' @include python.R
+#' @rdname alchemize
+#' @export
+alchemize_to_python.Dataset <- alchemize_to_python.arrow_dplyr_query <- function(x, ...) {

Review comment:
       You could define `r_to_py.Dataset` and `py_to_r.Dataset` like this, going through the RecordBatchReader (if/when that were useful, which it doesn't seem like it is for this issue).

##########
File path: r/R/alchemize.R
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Transform a data structure from one engine to another

Review comment:
       I see what you're trying to do here but I'm not convinced that we need it (yet, at least). 
   
   What if instead we defined `tbl.arrow_dplyr_query` et al.-- it is a generic in `dplyr`--so the tests would look like:
   
   ```r
   ds %>%
     select(int, lgl) %>%
     tbl(.engine = "duckdb") %>%
     summarize(...)
   ```
   

##########
File path: r/R/python.R
##########
@@ -15,6 +15,19 @@
 # specific language governing permissions and limitations
 # under the License.
 
+#' Transfer an Arrow memory structure to Python
+#'
+#' @param x the R-based Arrow object to export to Python
+#' @param ... arguments passed to the methods
+#'
+#' @retuen a reticulate-based Python object
+#'
+#' @keywords internal
+#' @export
+r_to_py <- function(x, ...) {

Review comment:
       I'm not sure this is a good idea because the `r_to_py` generic is in `reticulate`

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,91 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# TODO: note that this is copied from dbplyr

Review comment:
       I wouldn't say "copy" since this is clearly adapted, but do add a link to the inspiration




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675735126



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }

Review comment:
       πŸ‘ 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676618531



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)

Review comment:
       ```suggestion
       dplyr::summarise(to_duckdb(.data), ...)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676629389



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       Yeah, the pushdown will be here very shortly β€” I got an email from Pedro that he has a working version on a branch, though I haven't had a chance to test it myself yet. Regardless, I can't think of a reason _not_ to do this here anyway.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675581123



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A tbl of the new table in DuckDB
+#'
+#' @keywords internal
+#' @name to_duckdb
+NULL
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+.alchemize_to_duckdb <- function(

Review comment:
       πŸ‘ 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676626973



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       Good call, the way the duckdb implementation currently works, there's no pushdown of selection into the Arrow query from the duckdb side, so this optimization is something we should handle ourselves.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675077531



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,91 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# TODO: note that this is copied from dbplyr

Review comment:
       I don't think this needs LICENSE addition but you could also paraphrase it more so that it's less literally similar.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675054938



##########
File path: r/R/alchemize.R
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Transform a data structure from one engine to another

Review comment:
       Oh, yeah that's not too far off of what `tbl()` is used for elsewhere both `collect()` and `compute()` imply that there's a data processing/change step, where as `tbl()` doesn't have that same baggage. `tbl()` is a bit funny as it stands: [with `tbl.src_dbi()`](https://github.com/tidyverse/dbplyr/blob/47e53ce30402d41ae4b38c803de12e63d64a9b6c/R/src_dbi.R#L83-L86) takes a `src` (connection), and `from` a character of the table name versus the standard `tbl` from dplyr where the `src` argument is the thing you want to create a `tbl` from. Which would make our `tbl.*()` funny in that it has arguments: `src` - the Arrow object, `con` - the db connection to use, and `name` (or something like it, or maybe we embed that inside the function...) – the name of the view to create. 
   
   I think for now this is fine (though I will try and make sure that the docs explain the funny bit where _for arrow -> duckdb_ the `src` in `tbl` is *not* the duckdb dbi connection like one might expect (and instead there's a con argument for that)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676626973



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       Good call, the way the duckdb implementation currently works, there's no pushdown of selection into the Arrow query from the duckdb side, so this optimization is something we should handle ourselves.
   
   It might however be easier to add a TODO and defer this change to my arrow query engine PR because I'm rewriting the other half of this function there, and any thing we do here will probably get refactored too.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675691031



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,

Review comment:
       I might rename `src` to `.data` to be clear that we're working on the same dplyr inputs/objects.

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+# Creates an environment that disconnects the database when it's GC'd
+duckdb_disconnector <- function(con, tbl_name, quiet = FALSE) {

Review comment:
       This arg doesn't seem to be used
   
   ```suggestion
   duckdb_disconnector <- function(con, tbl_name) {
   ```

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)

Review comment:
       The first line in here should probably be `.data <- arrow_dplyr_query(.data)` to wrap/validate your input data.

##########
File path: r/_pkgdown.yml
##########
@@ -164,6 +164,9 @@ reference:
       - match_arrow
       - value_counts
       - list_compute_functions
+  - title: Connections to other systems
+    contents:
+      - to_duckdb

Review comment:
       Scope creep: should we manufacture a man page for the r_to_py/py_to_r methods?

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(

Review comment:
       Can you fix the indentation of the args to be consistent with the other function definitions (which AFAIK follow the tidyverse style guide)?

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }

Review comment:
       If you do wrap in `arrow_dplyr_query`, or if you have received one, `$group` isn't where the group vars are stored (though partial matching will probably save you). If you don't wrap in that/have a Table/Dataset/RecordBatch, `$group` won't be what you want either. 
   
   Reading our implementation for `dplyr::groups` I think this is what you want, not sure if the !! is needed.
   
   ```suggestion
     groups <- dplyr::groups(src)
     if (length(groups)) {
       tbl <- dplyr::group_by(tbl, !!groups)
     }
   ```

##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(
+  src,
+  con = arrow_duck_connection(),
+  table_name =  unique_arrow_tablename(),
+  auto_disconnect = TRUE) {
+  duckdb::duckdb_register_arrow(con, table_name, src)
+
+  tbl <- tbl(con, table_name)
+  groups <- src$group
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# Adapted from dbplyr
+unique_arrow_tablename <- function () {

Review comment:
       nit
   
   ```suggestion
   unique_arrow_tablename <- function() {
   ```

##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")

Review comment:
       Why not set this in the `options(arrow_duck_connection)`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675078920



##########
File path: r/R/alchemize.R
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Transform a data structure from one engine to another

Review comment:
       If `tbl()` doesn't seem quite right we don't need to force it. Easy enough to define a `to_duckdb(.data, con = arrow_duck_con(), ...)` function that behaves like we want.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676592945



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param .data the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(.data,
+                      con = arrow_duck_connection(),
+                      table_name =  unique_arrow_tablename(),
+                      auto_disconnect = TRUE) {
+  .data <- arrow_dplyr_query(.data)
+  duckdb::duckdb_register_arrow(con, table_name, .data)
+
+  tbl <- tbl(con, table_name)
+  groups <- dplyr::groups(.data)
+  if (length(groups)) {
+    tbl <- dplyr::group_by(tbl, groups)
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)
+  }
+
+  tbl
+}
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {

Review comment:
       Yeah, I agree, I'll dig a bit more and make an issue/PR with DBI




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675013509



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,192 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")
+
+test_that("basic integration", {

Review comment:
       Yes, I'll take it out




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r674925488



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,91 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+arrow_duck_connection <- function() {
+  con <- getOption("arrow_duck_con")
+  if (is.null(con) || !DBI::dbIsValid(con)) {
+    con <- DBI::dbConnect(duckdb::duckdb())
+    # Use the same CPU count that the arrow library is set to
+    DBI::dbExecute(con, paste0("PRAGMA threads=", cpu_count()))
+    options(arrow_duck_con = con)
+  }
+  con
+}
+
+# TODO: note that this is copied from dbplyr
+unique_arrow_tablename <- function () {
+  i <- getOption("arrow_table_name", 0) + 1
+  options(arrow_table_name = i)
+  sprintf("arrow_%03i", i)
+}
+
+.alchemize_to_duckdb <- function(x, ...) {
+  rb_to_duckdb(x, groups = x$group, ...)
+}
+
+rb_to_duckdb <- function(x, con = arrow_duck_connection(), groups = NULL, auto_disconnect = TRUE) {
+  table_name <- unique_arrow_tablename()
+  duckdb::duckdb_register_arrow(con, table_name, x)
+
+  tbl <- tbl(con, table_name)
+  if (length(groups) > 0 && !is.null(groups)) {
+    tbl <- dplyr::group_by(tbl, !!rlang::sym(groups))
+  }
+
+  if (auto_disconnect) {
+    # this will add the correct connection disconnection when the tbl is gced.
+    # we should probably confirm that this use of src$disco is kosher.
+    tbl$src$disco <- duckdb_disconnector(con, table_name)

Review comment:
       This setups up a specific cleanup step for how we register the table. This [is very similar to what dbplyr does](https://github.com/tidyverse/dbplyr/blob/47e53ce30402d41ae4b38c803de12e63d64a9b6c/R/src_dbi.R#L113) but might be reaching into the internals a bit too much (though works very very well in a way that deleting the table in `collect()` doesn't). If we go this route, I'll open an issue with dbplyr to ask if this is ok / if this is something that they want a PR to expose more for packages like ours.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675077691



##########
File path: r/DESCRIPTION
##########
@@ -40,9 +40,11 @@ Roxygen: list(markdown = TRUE, r6 = FALSE, load = "source")
 RoxygenNote: 7.1.1.9001
 VignetteBuilder: knitr
 Suggests:
+    DBI,
     decor,
     distro,
     dplyr,
+    duckdb (> 0.2.7),

Review comment:
       For the tests to run, yes. For now I added it as a step in the CI setup, since these tests _are_ pretty peripheral and for ~anyone installing any of the packages for real I'm certain they will already have dbplyr installed. But we can add it to suggests if we don't think that's too heavy.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675733244



##########
File path: r/R/duckdb.R
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#' Create a (virtual) DuckDB table from an Arrow object
+#'
+#' This will do the necessary configuration to create a (virtual) table in DuckDB
+#' that is backed by the Arrow object given. No data is copied or modified until
+#' `collect()` or `compute()` are called or a query is run against the table.
+#'
+#' The result is a dbplyr-compatible object that can be used in d(b)plyr pipelines.
+#'
+#' Alternatively, one can pass the argument `.engine = "duckdb"` to `summarise()`
+#' that starts with an Arrow object to use DuckDB to calculate the summarization
+#' step. Internally, this calls `to_duckdb()` with all of the default argument
+#' values.
+#'
+#' @param src the Arrow object (e.g. Dataset, Table) to use for the DuckDB table
+#' @param con a DuckDB connection to use (default will create one and store it
+#' in `options("arrow_duck_con")`)
+#' @param table_name a name to use in DuckDB for this object. The default is a
+#' unique string `"arrow_"` followed by numbers.
+#' @param auto_disconnect should the table be automatically cleaned up when the
+#' resulting object is removed (and garbage collected)? Default: `TRUE`
+#'
+#' @return A `tbl` of the new table in DuckDB
+#'
+#' @name to_duckdb
+#' @export
+#' @examplesIf arrow_with_dataset() && requireNamespace("duckdb", quietly = TRUE) && requireNamespace("dplyr", quietly = TRUE)
+#' library(dplyr)
+#'
+#' ds <- InMemoryDataset$create(mtcars)
+#'
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   to_duckdb() %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE))
+#'
+#' # the same query can be simplified using .engine = "duckdb"
+#' ds %>%
+#'   filter(mpg < 30) %>%
+#'   group_by(cyl) %>%
+#'   summarize(mean_mpg = mean(mpg, na.rm = TRUE), .engine = "duckdb")
+#'
+to_duckdb <- function(

Review comment:
       Yup, done. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane closed pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #10780:
URL: https://github.com/apache/arrow/pull/10780


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r676625386



##########
File path: r/R/dplyr-summarize.R
##########
@@ -18,19 +18,24 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
-summarise.arrow_dplyr_query <- function(.data, ...) {
-  call <- match.call()
-  .data <- arrow_dplyr_query(.data)
-  if (query_on_dataset(.data)) {
-    not_implemented_for_dataset("summarize()")
+summarise.arrow_dplyr_query <- function(.data, ..., .engine = c("arrow", "duckdb")) {
+  if (match.arg(.engine) == "duckdb") {
+    tbl <- to_duckdb(.data)
+    dplyr::summarise(tbl, ...)
+  } else {
+    call <- match.call()
+    .data <- arrow_dplyr_query(.data)
+    if (query_on_dataset(.data)) {
+      not_implemented_for_dataset("summarize()")
+    }
+    exprs <- quos(...)
+    # Only retain the columns we need to do our aggregations

Review comment:
       Good catch, let me re-arrange the if/else to do most of the work / this above it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10780: ARROW-12688: [R] Use DuckDB to query an Arrow Dataset

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10780:
URL: https://github.com/apache/arrow/pull/10780#discussion_r675129884



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+skip_if_not_installed("duckdb", minimum_version = "0.2.8")
+skip_if_not_installed("dbplyr")
+library(duckdb)
+library(dplyr)
+
+con <- dbConnect(duckdb::duckdb())
+# we always want to test in parallel
+dbExecute(con, "PRAGMA threads=2")
+
+# write one table to the connection so it is kept open
+DBI::dbWriteTable(con, "mtcars", mtcars)
+
+test_that("to_duckdb", {
+  ds <- InMemoryDataset$create(example_data)
+
+  expect_identical(
+    ds %>%
+      to_duckdb() %>%
+      collect() %>%
+      # factors don't roundtrip
+      select(!fct),
+    select(example_data, !fct)
+  )
+
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      to_duckdb() %>%
+      group_by(lgl) %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+
+  # can group_by before the to_duckdb
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      to_duckdb() %>%
+      summarise(mean_int = mean(int, na.rm = TRUE), mean_dbl = mean(dbl, na.rm = TRUE)) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("summarise(..., .engine)", {
+  ds <- InMemoryDataset$create(example_data)
+  expect_identical(
+    ds %>%
+      select(int, lgl, dbl) %>%
+      group_by(lgl) %>%
+      summarise(
+        mean_int = mean(int, na.rm = TRUE),
+        mean_dbl = mean(dbl, na.rm = TRUE),
+        .engine = "duckdb"
+      ) %>%
+      collect(),
+    tibble::tibble(
+      lgl = c(TRUE, NA, FALSE),
+      mean_int = c(3, 6.25, 8.5),
+      mean_dbl = c(3.1, 6.35, 6.1)
+    )
+  )
+})
+
+test_that("Joining, auto-cleanup", {
+  ds <- InMemoryDataset$create(example_data)
+
+  # we always want to test in parallel
+  dbExecute(con, "PRAGMA threads=2")
+
+  table_one <- to_duckdb(ds, con = con)
+  table_one_name <- as.character(table_one$ops$x)

Review comment:
       Thinking about the UX we want to advertise, would it be better to do something like this?
   
   ```suggestion
     table_one_name <- "table_one"
     table_one <- to_duckdb(ds, con = con, name = table_one_name)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org