From 64c65602f07a3a39686881d020dc23c20bc22305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 13 Mar 2025 07:51:48 +0100 Subject: [PATCH] feat: New `env` argument to `rel_from_sql()` --- R/cpp11.R | 4 ++-- R/relational.R | 4 ++-- R/rethrow-gen.R | 4 ++-- src/cpp11.cpp | 8 ++++---- src/relational.cpp | 20 +++++++++++++++++--- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/R/cpp11.R b/R/cpp11.R index e8c0e829a..15edc866b 100644 --- a/R/cpp11.R +++ b/R/cpp11.R @@ -148,8 +148,8 @@ rapi_rel_set_symdiff <- function(rel_a, rel_b) { .Call(`_duckdb_rapi_rel_set_symdiff`, rel_a, rel_b) } -rapi_rel_from_sql <- function(con, sql) { - .Call(`_duckdb_rapi_rel_from_sql`, con, sql) +rapi_rel_from_sql <- function(con, sql, env) { + .Call(`_duckdb_rapi_rel_from_sql`, con, sql, env) } rapi_rel_from_table <- function(con, schema_name, table_name) { diff --git a/R/relational.R b/R/relational.R index f0e291c11..263cf8669 100644 --- a/R/relational.R +++ b/R/relational.R @@ -480,8 +480,8 @@ rel_to_sql <- function(rel) { #' con <- DBI::dbConnect(duckdb()) #' DBI::dbWriteTable(con, "mtcars", mtcars) #' rel <- rel_from_sql(con, "SELECT * FROM mtcars") -rel_from_sql <- function(con, sql) { - rethrow_rapi_rel_from_sql(con@conn_ref, sql) +rel_from_sql <- function(con, sql, env = parent.frame()) { + rethrow_rapi_rel_from_sql(con@conn_ref, sql, env) } #' Create a duckdb table relation from a table name diff --git a/R/rethrow-gen.R b/R/rethrow-gen.R index f5760d891..a7506e4f7 100644 --- a/R/rethrow-gen.R +++ b/R/rethrow-gen.R @@ -333,9 +333,9 @@ rethrow_rapi_rel_set_symdiff <- function(rel_a, rel_b, call = parent.frame(2)) { ) } -rethrow_rapi_rel_from_sql <- function(con, sql, call = parent.frame(2)) { +rethrow_rapi_rel_from_sql <- function(con, sql, env, call = parent.frame(2)) { rlang::try_fetch( - rapi_rel_from_sql(con, sql), + rapi_rel_from_sql(con, sql, env), error = function(e) { rethrow_error_from_rapi(e, call) } diff --git a/src/cpp11.cpp b/src/cpp11.cpp index e1dc48df7..f5bb4994a 100644 --- a/src/cpp11.cpp +++ b/src/cpp11.cpp @@ -273,10 +273,10 @@ extern "C" SEXP _duckdb_rapi_rel_set_symdiff(SEXP rel_a, SEXP rel_b) { END_CPP11 } // relational.cpp -SEXP rapi_rel_from_sql(duckdb::conn_eptr_t con, const std::string sql); -extern "C" SEXP _duckdb_rapi_rel_from_sql(SEXP con, SEXP sql) { +SEXP rapi_rel_from_sql(duckdb::conn_eptr_t con, const std::string sql, SEXP env); +extern "C" SEXP _duckdb_rapi_rel_from_sql(SEXP con, SEXP sql, SEXP env) { BEGIN_CPP11 - return cpp11::as_sexp(rapi_rel_from_sql(cpp11::as_cpp>(con), cpp11::as_cpp>(sql))); + return cpp11::as_sexp(rapi_rel_from_sql(cpp11::as_cpp>(con), cpp11::as_cpp>(sql), cpp11::as_cpp>(env))); END_CPP11 } // relational.cpp @@ -458,7 +458,7 @@ static const R_CallMethodDef CallEntries[] = { {"_duckdb_rapi_rel_filter", (DL_FUNC) &_duckdb_rapi_rel_filter, 2}, {"_duckdb_rapi_rel_from_altrep_df", (DL_FUNC) &_duckdb_rapi_rel_from_altrep_df, 4}, {"_duckdb_rapi_rel_from_df", (DL_FUNC) &_duckdb_rapi_rel_from_df, 3}, - {"_duckdb_rapi_rel_from_sql", (DL_FUNC) &_duckdb_rapi_rel_from_sql, 2}, + {"_duckdb_rapi_rel_from_sql", (DL_FUNC) &_duckdb_rapi_rel_from_sql, 3}, {"_duckdb_rapi_rel_from_table", (DL_FUNC) &_duckdb_rapi_rel_from_table, 3}, {"_duckdb_rapi_rel_from_table_function", (DL_FUNC) &_duckdb_rapi_rel_from_table_function, 4}, {"_duckdb_rapi_rel_insert", (DL_FUNC) &_duckdb_rapi_rel_insert, 3}, diff --git a/src/relational.cpp b/src/relational.cpp index 3119df305..23495a543 100644 --- a/src/relational.cpp +++ b/src/relational.cpp @@ -4,6 +4,7 @@ #include "reltoaltrep.hpp" #include "R_ext/Random.h" +#include "httplib.hpp" #include "duckdb/parser/expression/columnref_expression.hpp" #include "duckdb/parser/expression/constant_expression.hpp" @@ -469,12 +470,25 @@ bool constant_expression_is_not_null(duckdb::expr_extptr_t expr) { // // DuckDB Relations: conversion -[[cpp11::register]] SEXP rapi_rel_from_sql(duckdb::conn_eptr_t con, const std::string sql) { +[[cpp11::register]] SEXP rapi_rel_from_sql(duckdb::conn_eptr_t con, const std::string sql, SEXP env) { if (!con || !con.get() || !con->conn) { - stop("rel_from_table: Invalid connection"); + stop("rel_from_sql: Invalid connection"); } + + D_ASSERT(con->db->env == R_NilValue); + con->db->env = (SEXP)env; + con->db->registered_dfs = Rf_cons(R_NilValue, R_NilValue); + duckdb_httplib::detail::scope_exit reset_db_env([&]() { + con->db->env = R_NilValue; + con->db->registered_dfs = R_NilValue; + }); + + // Possible FIXME (but anti-pattern): + // Prepend a WITH clause to the query to register the data frames. + // Better FIXME: RelationFromQuery() binds the data frames in the query. + auto rel = con->conn->RelationFromQuery(sql); - cpp11::writable::list prot = {}; + cpp11::writable::list prot = { con->db->registered_dfs }; return make_external_prot("duckdb_relation", prot, std::move(rel)); }