-
Notifications
You must be signed in to change notification settings - Fork 133
Pass Field information back and forth when using scalar UDFs #1299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@kosiew Here is an alternate approach. Instead of relying on extension type features it is going to pass the Field information when creating the FFI array. This will capture pyarrow extensions as well as any other metadata that any user assigns on the input. I'm going to leave it in draft until I can finish up those additional items on my check list. What do you think? cc @paleolimbot |
Definitely! Passing the argument fields/return fields should do it. Using We have a slightly different signature model in SedonaDB ("type matchers") because the existing signature matching doesn't consider metadata, but at the Arrow/FFI level we're doing approximately the same thing: apache/sedona-db#228 . We do use the concept of |
src/udf.rs
Outdated
| "_import_from_c", | ||
| ( | ||
| addr_of!(array) as Py_uintptr_t, | ||
| addr_of!(schema) as Py_uintptr_t, | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the use of PyArrow's private _import_from_c advisable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is a near duplicate of how we already convert ArrayData into a pyarrow object. You can see the original here. The difference in this function is that we know the field instead of only the data type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more modern way is to use __arrow_c_schema__ (although I think import_from_c will be around for a while). It's only a few lines:
https://github.com/apache/sedona-db/blob/main/python/sedonadb/src/import_from.rs#L151-L157
|
Opened new issue so there isn't too much scope creep |
Which issue does this PR close?
Closes #1172
Rationale for this change
Since we now have the ability to pass Field information instead of just DataType with ScalarUDFs, this feature adds similar support for Python written UDFs. Without this feature you must write your UDFs in rust and expose them to Python. This enhancement greatly expands the use cases where PyArrow data can be leveraged.
What changes are included in this PR?
create_udffunctionAre there any user-facing changes?
This expands on the current API and is backwards compatible.
TODO