-
Notifications
You must be signed in to change notification settings - Fork 599
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
feat(udf): add initial support for JavaScript UDF #14513
Conversation
Sounds exciting! QQ: why this is going to be merged into |
Just don't want to resolve conflicts with that branch because I suppose the WASM PR will be merged soon. 🥵 |
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.
Rest LGTM!
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.
Rest LGTM
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
b774e8b
to
23599c3
Compare
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
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.
Rest LGTM
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
3121e9a
to
a341309
Compare
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.
Generally LGTM
Signed-off-by: Runji Wang <[email protected]>
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.
(Already discussed with Runji, also comment here)
The arrow conversion before calling js udf doesn't make total sense to me. Under the hood, arrow_udf_js::Runtime
simply iterate over the rows of the RecordBatch, and then convert the row to js obj, and then eval with js func. And RisingWave chunk -> Arrow RecordBatch isn't zero-cost neither. So basically it just uses arrow's type system as the interface. Alternatively we can directly convert RisingWave's Row to js obj. The additional Arrow conversion has performance overheads and it also doesn't make implementation simpler. It's unlike external UDF and WASM UDF, where the Arrow data is passed to the guest for better performance, and end-users work with Arrow data directly. Arrow format is actually transparent to users for js udf here.
arrow_udf_js
is like when you already have Arrow data, and how to do js udf with it. The rationale that this can be used by other ppl and benefit (and benefit from) the ecosystem is somewhat reasonable. Besides, I don't care performance that much and when we want to optimize it we can still save the arrow conversion without any breakage. So this is acceptable to me, and I prefer to ship this and let users try it.
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR adds an experimental support for JavaScript UDF.
In this PR, we support scalar functions and table functions with all types except temporal types.
The JavaScript code is inlined in the create-function statement, stored in our catalog and run in an embedded QuickJS runtime rquickjs. I have verified that the runtime is sandboxed, users can do nothing but pure calculations. (QuickJS provides an std library including basic functions such as
print
andconsole.log
in interpretation mode. But it's not available in the embedded mode.) You can review the arrow-udf-js crate to see how the runtime works.In terms of performance, a simple test shows that JavaScript functions are ~60x slower than native functions.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
Adds an experimental support for JavaScript UDF.
See src/expr/udf/README-js.md for full documents.