[R] arrow_eval: do we need both nse_funcs and .cache$functions?

[asfimport]

Currently we have 2 copies of the same information, once in nse_funcs and once in {}.cache$functions{}. I wasn't able to figure out the reason for this. Maybe I am missing something or maybe this is just legacy code that we can update.   

Reporter: Dragoș Moldovan-Grünfeld / @dragosmg

_{Note: This issue was originally created as ARROW-17495. Please see the migration documentation for further details.}

[asfimport]

Nicola Crane / @thisisnic:
@dragosmg I was trying to look into this, and saw this in the docs for the update_cache param for register_binding():

"The reason why .cache$functions is needed in addition to nse_funcs for non-aggregate functions could be revisited...it is currently used as the data mask in mutate, filter, and aggregate (but not summarise) because the data mask has to be a list."

Given that @paleolimbot made the PR for the function registry stuff, he might be the person to ask about this?

[asfimport]

Dragoș Moldovan-Grünfeld / @dragosmg:
Thanks for the pointer. I raised this ticket after a conversation with him as we were both wondering why the duplication existed.

[asfimport]

Dewey Dunnington / @paleolimbot:
Dragos got here first, but for the public record, I have no idea why it's there and I think it should be removed, personally (unless somebody pipes up here with the reason for its existence).

[asfimport]

Neal Richardson / @nealrichardson:
Per https://github.com/apache/arrow/blob/master/r/R/arrow-package.R#L64-L67, nse_funcs can't be fully populated until load time.

Feel free to try removing it and seeing what breaks, and be sure to check benchmarks--I don't know how much overhead there is in re-creating a bunch of function definitions every time you arrow_eval, maybe it's a lot and maybe it isn't.

[asfimport]

Dragoș Moldovan-Grünfeld / @dragosmg:
I can look into it (maybe after I finish with the capstone).

As far as I can tell, both .cache$functions and nse_funcs are created at load time. cache$functions is nse_funcs + Arrow Compute functions (prefixed with {}arrow_{}).
I bumped into this while trying to register / translate a user-defined function with pre-existing bindings. I needed to update either cache.functions or nse_funcs - we can update the former via {}update_cache = TRUE{}, but then I had to change call_binding() to fetch from the updated cache and not from {}nse_funcs{}. This led me to think that folks might be confused by these 2 objects that overlap by quite a bit and are in some situations interchangeable (mostly nse_funcs can be replaced by cache$functions which includes it). 

[github-actions]

This issue has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this issue will be closed in 14 days. If this improvement is still desired but has no current owner, please add the 'Status: needs champion' label.