diff --git a/R/cpp11.R b/R/cpp11.R index bca451c89..5b6dc6c15 100644 --- a/R/cpp11.R +++ b/R/cpp11.R @@ -172,12 +172,17 @@ rapi_rel_from_table_function <- function(con, function_name, positional_paramete .Call(`_duckdb_rapi_rel_from_table_function`, con, function_name, positional_parameters_sexps, named_parameters_sexps) } -rapi_rel_to_altrep <- function(rel) { - .Call(`_duckdb_rapi_rel_to_altrep`, rel) +rapi_get_last_rel_mat <- function() { + .Call(`_duckdb_rapi_get_last_rel_mat`) } -rapi_rel_from_altrep_df <- function(df, strict, allow_materialized) { - .Call(`_duckdb_rapi_rel_from_altrep_df`, df, strict, allow_materialized) +# allow_materialization = TRUE: compatibility with duckplyr <= 0.4.1 +rapi_rel_to_altrep <- function(rel, allow_materialization = TRUE) { + .Call(`_duckdb_rapi_rel_to_altrep`, rel, allow_materialization) +} + +rapi_rel_from_altrep_df <- function(df, strict, allow_materialized, enable_materialization) { + .Call(`_duckdb_rapi_rel_from_altrep_df`, df, strict, allow_materialized, enable_materialization) } rapi_release <- function(stmt) { diff --git a/R/relational.R b/R/relational.R index edcd931d3..9c965193f 100644 --- a/R/relational.R +++ b/R/relational.R @@ -382,13 +382,18 @@ rel_set_alias <- function(rel, alias) { #' con <- DBI::dbConnect(duckdb()) #' rel <- rel_from_df(con, mtcars) #' print(rel_to_altrep(rel)) -rel_to_altrep <- function(rel) { - rethrow_rapi_rel_to_altrep(rel) +rel_to_altrep <- function(rel, allow_materialization = TRUE) { + rethrow_rapi_rel_to_altrep(rel, allow_materialization) } #' Retrieves the data frame back from a altrep df #' @param df the data frame created by rel_to_altrep +#' @param strict whether to throw an error if the data frame is not an altrep +#' or if other criteria are not met +#' @param allow_materialized whether to succeed if the data frame is already materialized +#' @param enable_materialization set to `TRUE` for side effect: allow materialization of this a data frame +#' if it was not allowed previously #' @return the relation object #' @noRd #' @examples @@ -396,8 +401,8 @@ rel_to_altrep <- function(rel) { #' rel <- rel_from_df(con, mtcars) #' df = rel_to_altrep(rel) #' print(rel_from_altrep_df(df)) -rel_from_altrep_df <- function(df, strict = TRUE, allow_materialized = TRUE) { - rethrow_rapi_rel_from_altrep_df(df, strict, allow_materialized) +rel_from_altrep_df <- function(df, strict = TRUE, allow_materialized = TRUE, enable_materialization = FALSE) { + rethrow_rapi_rel_from_altrep_df(df, strict, allow_materialized, enable_materialization) } @@ -470,3 +475,7 @@ rel_names <- function(rel) { load_rfuns <- function() { rethrow_rapi_load_rfuns() } + +get_last_rel_mat <- function() { + rethrow_rapi_get_last_rel_mat() +} diff --git a/R/rethrow-gen.R b/R/rethrow-gen.R index 3802f1809..7c59dbb82 100644 --- a/R/rethrow-gen.R +++ b/R/rethrow-gen.R @@ -387,18 +387,28 @@ rethrow_rapi_rel_from_table_function <- function(con, function_name, positional_ ) } -rethrow_rapi_rel_to_altrep <- function(rel, call = parent.frame(2)) { +rethrow_rapi_get_last_rel_mat <- function(call = parent.frame(2)) { rlang::try_fetch( - rapi_rel_to_altrep(rel), + rapi_get_last_rel_mat(), error = function(e) { rethrow_error_from_rapi(e, call) } ) } -rethrow_rapi_rel_from_altrep_df <- function(df, strict, allow_materialized, call = parent.frame(2)) { +rethrow_rapi_rel_to_altrep <- function(rel, allow_materialization = TRUE, call = parent.frame(2)) { rlang::try_fetch( - rapi_rel_from_altrep_df(df, strict, allow_materialized), + # duckplyr compat + rapi_rel_to_altrep(rel, allow_materialization), + error = function(e) { + rethrow_error_from_rapi(e, call) + } + ) +} + +rethrow_rapi_rel_from_altrep_df <- function(df, strict, allow_materialized, enable_materialization, call = parent.frame(2)) { + rlang::try_fetch( + rapi_rel_from_altrep_df(df, strict, allow_materialized, enable_materialization), error = function(e) { rethrow_error_from_rapi(e, call) } @@ -575,6 +585,7 @@ rethrow_restore <- function() { rethrow_rapi_rel_from_sql <<- rapi_rel_from_sql rethrow_rapi_rel_from_table <<- rapi_rel_from_table rethrow_rapi_rel_from_table_function <<- rapi_rel_from_table_function + rethrow_rapi_get_last_rel_mat <<- rapi_get_last_rel_mat rethrow_rapi_rel_to_altrep <<- rapi_rel_to_altrep rethrow_rapi_rel_from_altrep_df <<- rapi_rel_from_altrep_df rethrow_rapi_release <<- rapi_release diff --git a/src/cpp11.cpp b/src/cpp11.cpp index 1079b3dc9..f51838417 100644 --- a/src/cpp11.cpp +++ b/src/cpp11.cpp @@ -114,7 +114,7 @@ extern "C" SEXP _duckdb_rapi_expr_constant(SEXP val) { SEXP rapi_expr_comparison(list exprs, std::string cmp_op); extern "C" SEXP _duckdb_rapi_expr_comparison(SEXP exprs, SEXP cmp_op) { BEGIN_CPP11 - return cpp11::as_sexp(rapi_expr_comparison(cpp11::as_cpp>(exprs),cpp11::as_cpp>(cmp_op))); + return cpp11::as_sexp(rapi_expr_comparison(cpp11::as_cpp>(exprs), cpp11::as_cpp>(cmp_op))); END_CPP11 } // relational.cpp @@ -315,17 +315,24 @@ extern "C" SEXP _duckdb_rapi_rel_from_table_function(SEXP con, SEXP function_nam END_CPP11 } // reltoaltrep.cpp -SEXP rapi_rel_to_altrep(duckdb::rel_extptr_t rel); -extern "C" SEXP _duckdb_rapi_rel_to_altrep(SEXP rel) { +std::string rapi_get_last_rel_mat(); +extern "C" SEXP _duckdb_rapi_get_last_rel_mat() { BEGIN_CPP11 - return cpp11::as_sexp(rapi_rel_to_altrep(cpp11::as_cpp>(rel))); + return cpp11::as_sexp(rapi_get_last_rel_mat()); END_CPP11 } // reltoaltrep.cpp -SEXP rapi_rel_from_altrep_df(SEXP df, bool strict, bool allow_materialized); -extern "C" SEXP _duckdb_rapi_rel_from_altrep_df(SEXP df, SEXP strict, SEXP allow_materialized) { +SEXP rapi_rel_to_altrep(duckdb::rel_extptr_t rel, bool allow_materialization); +extern "C" SEXP _duckdb_rapi_rel_to_altrep(SEXP rel, SEXP allow_materialization) { BEGIN_CPP11 - return cpp11::as_sexp(rapi_rel_from_altrep_df(cpp11::as_cpp>(df), cpp11::as_cpp>(strict), cpp11::as_cpp>(allow_materialized))); + return cpp11::as_sexp(rapi_rel_to_altrep(cpp11::as_cpp>(rel), cpp11::as_cpp>(allow_materialization))); + END_CPP11 +} +// reltoaltrep.cpp +SEXP rapi_rel_from_altrep_df(SEXP df, bool strict, bool allow_materialized, bool enable_materialization); +extern "C" SEXP _duckdb_rapi_rel_from_altrep_df(SEXP df, SEXP strict, SEXP allow_materialized, SEXP enable_materialization) { + BEGIN_CPP11 + return cpp11::as_sexp(rapi_rel_from_altrep_df(cpp11::as_cpp>(df), cpp11::as_cpp>(strict), cpp11::as_cpp>(allow_materialized), cpp11::as_cpp>(enable_materialization))); END_CPP11 } // statement.cpp @@ -445,6 +452,7 @@ static const R_CallMethodDef CallEntries[] = { {"_duckdb_rapi_expr_set_alias", (DL_FUNC) &_duckdb_rapi_expr_set_alias, 2}, {"_duckdb_rapi_expr_tostring", (DL_FUNC) &_duckdb_rapi_expr_tostring, 1}, {"_duckdb_rapi_expr_window", (DL_FUNC) &_duckdb_rapi_expr_window, 9}, + {"_duckdb_rapi_get_last_rel_mat", (DL_FUNC) &_duckdb_rapi_get_last_rel_mat, 0}, {"_duckdb_rapi_get_null_SEXP_ptr", (DL_FUNC) &_duckdb_rapi_get_null_SEXP_ptr, 0}, {"_duckdb_rapi_get_substrait", (DL_FUNC) &_duckdb_rapi_get_substrait, 3}, {"_duckdb_rapi_get_substrait_json", (DL_FUNC) &_duckdb_rapi_get_substrait_json, 3}, @@ -464,7 +472,7 @@ static const R_CallMethodDef CallEntries[] = { {"_duckdb_rapi_rel_distinct", (DL_FUNC) &_duckdb_rapi_rel_distinct, 1}, {"_duckdb_rapi_rel_explain", (DL_FUNC) &_duckdb_rapi_rel_explain, 1}, {"_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, 3}, + {"_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_table", (DL_FUNC) &_duckdb_rapi_rel_from_table, 3}, @@ -479,7 +487,7 @@ static const R_CallMethodDef CallEntries[] = { {"_duckdb_rapi_rel_set_intersect", (DL_FUNC) &_duckdb_rapi_rel_set_intersect, 2}, {"_duckdb_rapi_rel_set_symdiff", (DL_FUNC) &_duckdb_rapi_rel_set_symdiff, 2}, {"_duckdb_rapi_rel_sql", (DL_FUNC) &_duckdb_rapi_rel_sql, 2}, - {"_duckdb_rapi_rel_to_altrep", (DL_FUNC) &_duckdb_rapi_rel_to_altrep, 1}, + {"_duckdb_rapi_rel_to_altrep", (DL_FUNC) &_duckdb_rapi_rel_to_altrep, 2}, {"_duckdb_rapi_rel_to_df", (DL_FUNC) &_duckdb_rapi_rel_to_df, 1}, {"_duckdb_rapi_rel_to_parquet", (DL_FUNC) &_duckdb_rapi_rel_to_parquet, 2}, {"_duckdb_rapi_rel_to_sql", (DL_FUNC) &_duckdb_rapi_rel_to_sql, 1}, diff --git a/src/reltoaltrep.cpp b/src/reltoaltrep.cpp index d65645543..08680eb6d 100644 --- a/src/reltoaltrep.cpp +++ b/src/reltoaltrep.cpp @@ -74,7 +74,7 @@ static T *GetFromExternalPtr(SEXP x) { } auto wrapper = (T *)R_ExternalPtrAddr(ptr); if (!wrapper) { - cpp11::stop("This looks like it has been freed"); + cpp11::stop("GetFromExternalPtr: This looks like it has been freed"); } return wrapper; } @@ -85,7 +85,8 @@ struct AltrepRelationWrapper { return GetFromExternalPtr(x); } - AltrepRelationWrapper(duckdb::shared_ptr rel_p) : rel(rel_p) { + AltrepRelationWrapper(duckdb::shared_ptr rel_p, bool allow_materialization_) + : rel(rel_p), allow_materialization(allow_materialization_) { } bool HasQueryResult() const { @@ -94,11 +95,17 @@ struct AltrepRelationWrapper { MaterializedQueryResult *GetQueryResult() { if (!res) { + if (!allow_materialization) { + cpp11::stop("Materialization is disabled, use collect() or as_tibble() to materialize"); + } + auto option = Rf_GetOption(RStrings::get().materialize_sym, R_BaseEnv); if (option != R_NilValue && !Rf_isNull(option) && LOGICAL_ELT(option, 0) == true) { - Rprintf("materializing:\n%s\n", rel->ToString().c_str()); + Rprintf("duckplyr: materializing, review details with duckplyr::last_rel_mat()\n"); } + last_rel = rel; + ScopedInterruptHandler signal_handler(rel->context.GetContext()); // We need to temporarily allow a deeper execution stack @@ -133,10 +140,16 @@ struct AltrepRelationWrapper { return (MaterializedQueryResult *)res.get(); } + bool allow_materialization; + duckdb::shared_ptr rel; duckdb::unique_ptr res; + + static duckdb::shared_ptr last_rel; }; +duckdb::shared_ptr AltrepRelationWrapper::last_rel; + struct AltrepRownamesWrapper { AltrepRownamesWrapper(duckdb::shared_ptr rel_p) : rel(rel_p) { @@ -325,12 +338,19 @@ static R_altrep_class_t LogicalTypeToAltrepType(const LogicalType &type) { } } -[[cpp11::register]] SEXP rapi_rel_to_altrep(duckdb::rel_extptr_t rel) { +[[cpp11::register]] std::string rapi_get_last_rel_mat() { + if (!AltrepRelationWrapper::last_rel) { + return ""; + } + return AltrepRelationWrapper::last_rel->ToString(); +} + +[[cpp11::register]] SEXP rapi_rel_to_altrep(duckdb::rel_extptr_t rel, bool allow_materialization) { D_ASSERT(rel && rel->rel); auto drel = rel->rel; auto ncols = drel->Columns().size(); - auto relation_wrapper = make_shared_ptr(drel); + auto relation_wrapper = make_shared_ptr(drel, allow_materialization); cpp11::writable::list data_frame; data_frame.reserve(ncols); @@ -367,7 +387,7 @@ static R_altrep_class_t LogicalTypeToAltrepType(const LogicalType &type) { return data_frame; } -[[cpp11::register]] SEXP rapi_rel_from_altrep_df(SEXP df, bool strict = true, bool allow_materialized = true) { +[[cpp11::register]] SEXP rapi_rel_from_altrep_df(SEXP df, bool strict, bool allow_materialized, bool enable_materialization) { if (!Rf_inherits(df, "data.frame")) { if (strict) { cpp11::stop("rapi_rel_from_altrep_df: Not a data.frame"); @@ -403,9 +423,8 @@ static R_altrep_class_t LogicalTypeToAltrepType(const LogicalType &type) { } } + auto wrapper = GetFromExternalPtr(row_names); if (!allow_materialized) { - auto wrapper = GetFromExternalPtr(row_names); - if (wrapper->rel->res.get()) { // We return NULL here even for strict = true // because this is expected from df_is_materialized() @@ -422,6 +441,12 @@ static R_altrep_class_t LogicalTypeToAltrepType(const LogicalType &type) { } } + // Side effect comes last + // FIXME: Add separate rapi_() function for this + if (enable_materialization) { + wrapper->rel->allow_materialization = true; + } + return res; }