feat: rapi_get_last_rel_mat(), `rapi_rel_to_altrep(allow_materializ…

…ation = TRUE)`, `rapi_rel_from_altrep_df(enable_materialization)` (#526)

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) {

R/relational.R

@@ -382,22 +382,27 @@ 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
 #' con <- DBI::dbConnect(duckdb())
 #' 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()
+}

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

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<cpp11::decay_t<list>>(exprs),cpp11::as_cpp<cpp11::decay_t<std::string>>(cmp_op)));
+    return cpp11::as_sexp(rapi_expr_comparison(cpp11::as_cpp<cpp11::decay_t<list>>(exprs), cpp11::as_cpp<cpp11::decay_t<std::string>>(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<cpp11::decay_t<duckdb::rel_extptr_t>>(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<cpp11::decay_t<SEXP>>(df), cpp11::as_cpp<cpp11::decay_t<bool>>(strict), cpp11::as_cpp<cpp11::decay_t<bool>>(allow_materialized)));
+    return cpp11::as_sexp(rapi_rel_to_altrep(cpp11::as_cpp<cpp11::decay_t<duckdb::rel_extptr_t>>(rel), cpp11::as_cpp<cpp11::decay_t<bool>>(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<cpp11::decay_t<SEXP>>(df), cpp11::as_cpp<cpp11::decay_t<bool>>(strict), cpp11::as_cpp<cpp11::decay_t<bool>>(allow_materialized), cpp11::as_cpp<cpp11::decay_t<bool>>(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},

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<AltrepRelationWrapper>(x);
 	}
 
-	AltrepRelationWrapper(duckdb::shared_ptr<Relation> rel_p) : rel(rel_p) {
+	AltrepRelationWrapper(duckdb::shared_ptr<Relation> 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<Relation> rel;
 	duckdb::unique_ptr<QueryResult> res;
+
+	static duckdb::shared_ptr<Relation> last_rel;
 };
 
+duckdb::shared_ptr<Relation> AltrepRelationWrapper::last_rel;
+
 struct AltrepRownamesWrapper {
 
 	AltrepRownamesWrapper(duckdb::shared_ptr<AltrepRelationWrapper> 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<AltrepRelationWrapper>(drel);
+	auto relation_wrapper = make_shared_ptr<AltrepRelationWrapper>(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<AltrepRownamesWrapper>(row_names);
 	if (!allow_materialized) {
-		auto wrapper = GetFromExternalPtr<AltrepRownamesWrapper>(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;
 }