[C++][Python] Unregister compute functions

[asfimport]

In general, when using UDFs, the user defines a function expecting a particular outcome. When building the program, there needs to be a way to update existing function kernels if it expands beyond what is planned before. In such situations, there should be a way to remove the existing definition and add a new definition. To enable this, the unregister functionality has to be included. 

Reporter: Vibhatha Lakmal Abeykoon / @vibhatha
Assignee: Vibhatha Lakmal Abeykoon / @vibhatha

Related issues:

• [C++] Arrow Substrait enhancements for UDF (relates to)

PRs and other links:

• GitHub Pull Request #14308

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

[asfimport]

Weston Pace / @westonpace:
Another task we should tackle as part of this PR that came up during the discussion of ARROW-15639: #12590 (comment)

If python is finalized then any python UDFs that have been registered will be invalid. We should consider adding an atexit hook to pyarrow which unregisters all python UDFs.

This is something of a corner case, as finalization usually happens at process exit, but in some cases users may have multiple python interpreters or be restarting the python interpreter.

[asfimport]

Antoine Pitrou / @pitrou:
Yes, an atexit hook is probably the way to go. It should be easy to do in Cython code.

[asfimport]

Yaron Gvili / @rtpsw:
Another alternative to consider is registering Python UDFs to an extension registry instance that (1) is specific to the Python interpreter and (2) is linked to the default global one (so it can find both UDF and normal functions). This Python-specific registry would then be passed to be used by the execution engine. I think this way (only) the Python-specific registry would naturally get cleaned up on finalization of the Python interpreter.

[asfimport]

Yaron Gvili / @rtpsw:
This second-layer-registry approach is good for another use case in which the user runs multiple execution engine invocations, either in sequence or in parallel, from the same Python interpreter and wants to keep separate the UDFs registered in each invocation.

[asfimport]

Yaron Gvili / @rtpsw:
I created a PR for a nested extension-id-registry using the approach I proposed. Please let me know what you think, as I have my own use cases for this nested extension-registry-id.

[asfimport]

Yaron Gvili / @rtpsw:
See this PR as a solution for this comment.

[asfimport]

Todd Farmer / @toddfarmer:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned per project policy. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.

[asfimport]

Yaron Gvili / @rtpsw:
I believe this issue is superseded by ARROW-16677 which allows creating a nested registry that can later be dropped lock-free along with all functions registered on it.

[asfimport]

Vibhatha Lakmal Abeykoon / @vibhatha:
@rtpsw I still need to look into this and verify. Thanks for the note.

[asfimport]

Vibhatha Lakmal Abeykoon / @vibhatha:
@rtpsw  definitely this would be helpful in cleaning up things per interpreter. I am curious about this case. 

Imagine there is a UDF created in Python which does a particular job, but next in the middle of the program we need that function to have a different behavior (meaning another function needs to replace this function). So we expect the same function name, but a different functionality. In such a case we still have to remove an existing function with the same name and function signature. We cannot check the logic of the already registered function and the new function (in my understanding). 

On top of my head what I imagine is an active interpreter, where we do a demo of UDFs. I create a function called "func" and write some functionality. Then I want to write another example in the same interpreter instance with the same function name but a different functionality itself. Would this encourage deleting the function explicitly?

Please correct me if I am wrong.

cc @westonpace  

[asfimport]

Yaron Gvili / @rtpsw:
I think many PyArrow function execution code paths lead to Arrow C++ APIs with a configurable function registry. For example, the CallFunction API accepts a function registry via its ExecContext argument. So, one could create a first nested function registry holding a first implementation of a UDF, use this first registry for several function execution invocations, then create a second nested function registry holding a second implementation of the UDF, and switch over to using this second registry. The Python interpreter remains the same, and there's no need to remove the UDF - the first registry can just be dropped.

[asfimport]

Vibhatha Lakmal Abeykoon / @vibhatha:
What if subset of functions registered in that particular registry is needed and we just need another subset of functions. Then we have to re-register them. What about that situation?

[asfimport]

Vibhatha Lakmal Abeykoon / @vibhatha:
cc @rtpsw @westonpace  

[asfimport]

Yaron Gvili / @rtpsw:
Consider a case where X, Y, and Z are disjoint sets of functions. One can register (the functions of) X in registry 1, then register Y in registry 2 whose parent is registry 1, and finally register Z in registry 3 whose parent is registry 2. With respect to the case described by Vibhata, I believe the default registry corresponds to registry 1, the first subset of functions corresponds to Y, and the second to Z. This works nicely when X, Y, and Z are known upfront; otherwise, one may need to register again. For example, suppose Y1 and Y2 are disjoint sets of functions whose union is Y, and one wants to register functions on top of those of X and Y1, then one would need to create a fresh registry whose parent is registry 1, and register Y1 again on this fresh registry. I think this is a reasonable result when not knowing upfront. In general, considering registries have extended scopes, I think it is better to create a fresh nested registry and keep it fixed while in use than to edit an existing one using removals while in use.

[asfimport]

Vibhatha Lakmal Abeykoon / @vibhatha:
Yes this is a good point. I think if we go on this route, we must make sure we properly define these things in docs or provide examples of using the UDFs with a few diverse examples covering these angles. Eitherway, I personally like the nested registry idea, it gives the flexibility decide these things prior to writing the programme. In general, we could assume one would put required effort to clarify these and then write the programme. I think this is fair enough to proceed with this approach. 

Also, we may need to include a few test cases on behalf of this and test the idea and make sure if things are not exposed to Python to create the nested registries as well. I am still browsing through the said change (nested registries). Please correct me if I am wrong.

cc @westonpace  this approach sounds fair enough for to proceed. Appreciate your thoughts.