Inital stab at lazy data frame scan for R #164
Conversation
|
Needs still tests and some protect stuff I guess |
| return nullptr; | ||
| } | ||
| if (TYPEOF(df) == PROMSXP) { | ||
| df = Rf_eval(df, rho); |
There was a problem hiding this comment.
cpp11 has a global protection mechanism, but how to control the lifetime?
There was a problem hiding this comment.
indeed lifetime is a problematic issue. For example, once someone creates a view of this and then deletes the df and gc runs we might be in trouble.
There was a problem hiding this comment.
IIUC, support for lifetime control has landed via duckdb/duckdb#11761, which is why we now have conflicts here?
There was a problem hiding this comment.
We already had support for lifetime control, but it was broken in a couple ways
We use the ExternalDependency class for lifetime control, but this wasn't being kept alive long enough, which this PR aimed to address.
We changed the prototype of the replacement scan, we changed the const string &table_name into ReplacementScanInput
I have a follow up PR scheduled that removes the newly added table_ref &TableRef from this struct though
Instead we will collect replacement scans inside QueryRelation::Bind (virtual overload) and push these replacement scans as CTEs onto the parsed QueryNode stored inside the QueryRelation.
There was a problem hiding this comment.
Thanks, Thijs. May I get back to you for help with lifetime control after that follow-up PR has made it into the R package? I don't find that PR in https://github.com/duckdb/duckdb/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc+author%3ATishj.
There was a problem hiding this comment.
@Tishj: how would I go about controlling the lifetime of the bound object in this PR?
| return nullptr; | ||
| } | ||
| // TODO: do utf conversion | ||
| auto table_function = make_uniq<TableFunctionRef>(); |
There was a problem hiding this comment.
Or perhaps subclass TableFunctionRef ?
now "just" works
CC @krlmlr