From 21a09b3ee2731987e9c294e1902d8d164ad8b383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 10 Apr 2025 07:42:46 +0200 Subject: [PATCH 1/2] feat: Reuse intermediate materialization results --- R/relational-duckdb.R | 8 ++++++- tests/testthat/_snaps/relational-duckdb.md | 11 +++++----- tests/testthat/test-relational-duckdb.R | 25 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/R/relational-duckdb.R b/R/relational-duckdb.R index 063d33dd5..845cbe604 100644 --- a/R/relational-duckdb.R +++ b/R/relational-duckdb.R @@ -99,7 +99,13 @@ duckdb_rel_from_df <- function(df, call = caller_env()) { # FIXME: make generic stopifnot(is.data.frame(df)) - rel <- duckdb$rel_from_altrep_df(df, strict = FALSE, allow_materialized = FALSE) + rel <- duckdb$rel_from_altrep_df( + df, + strict = FALSE, + allow_materialized = FALSE, + wrap = TRUE + ) + if (!is.null(rel)) { # Once we're here, we know it's an ALTREP data frame # We don't get here if it's already materialized diff --git a/tests/testthat/_snaps/relational-duckdb.md b/tests/testthat/_snaps/relational-duckdb.md index 62bb68a10..c3c41b559 100644 --- a/tests/testthat/_snaps/relational-duckdb.md +++ b/tests/testthat/_snaps/relational-duckdb.md @@ -42,11 +42,12 @@ --------------------- --- Relation Tree --- --------------------- - Projection [a as a] - Order [___row_number ASC] - Filter [(a = 1.0)] - Projection [a as a, row_number() OVER () as ___row_number] - r_dataframe_scan(0xdeadbeef) + AltrepDataFrame [0xdeadbeef] + Projection [a as a] + Order [___row_number ASC] + Filter [(a = 1.0)] + Projection [a as a, row_number() OVER () as ___row_number] + r_dataframe_scan(0xdeadbeef) --------------------- -- Result Columns -- diff --git a/tests/testthat/test-relational-duckdb.R b/tests/testthat/test-relational-duckdb.R index 4df078a3a..3864ecf1a 100644 --- a/tests/testthat/test-relational-duckdb.R +++ b/tests/testthat/test-relational-duckdb.R @@ -138,3 +138,28 @@ test_that("duckdb_rel_from_df() uses materialized results", { expect_equal(n_calls, 1) }) + +test_that("duckdb_rel_from_df() uses materialized intermediate results", { + skip_if(identical(Sys.getenv("R_COVR"), "true")) + + withr::local_envvar(DUCKPLYR_OUTPUT_ORDER = FALSE) + + df1 <- duckdb_tibble(a = 1) + df2 <- df1 |> arrange(a) + df3 <- df2 |> mutate(b = 2) + + rel2 <- duckdb:::rel_from_altrep_df(df2, wrap = TRUE) + expect_length(strsplit(duckdb:::rel_tostring(rel2, "tree"), "\n")[[1]], 4) + + rel3 <- duckdb:::rel_from_altrep_df(df3, wrap = TRUE) + expect_length(strsplit(duckdb:::rel_tostring(rel3, "tree"), "\n")[[1]], 6) + + # Side effect: trigger intermediate materialization + nrow(df2) + + # The depth of the rel2 tree is shorter thanks to `wrap = TRUE` + expect_length(strsplit(duckdb:::rel_tostring(rel2, "tree"), "\n")[[1]], 2) + + # The depth of the rel3 tree is shorter now too + expect_length(strsplit(duckdb:::rel_tostring(rel3, "tree"), "\n")[[1]], 4) +}) From b030a919e7320d2191fff667c52e17327e4db384 Mon Sep 17 00:00:00 2001 From: krlmlr <1741643+krlmlr@users.noreply.github.com> Date: Thu, 10 Apr 2025 05:48:04 +0000 Subject: [PATCH 2/2] [create-pull-request] automated change --- tests/testthat/_snaps/compute.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/compute.md b/tests/testthat/_snaps/compute.md index 87d426ba1..e4a052c97 100644 --- a/tests/testthat/_snaps/compute.md +++ b/tests/testthat/_snaps/compute.md @@ -7,7 +7,8 @@ --------------------- --- Relation Tree --- --------------------- - Scan Table [duckplyr_4hYuvhNS26] + AltrepDataFrame [0x55d9090e7d18] + Scan Table [duckplyr_4hYuvhNS26] --------------------- -- Result Columns --