feat(udf): Add deno as UDF language #15719

bakjos · 2024-03-15T16:36:26Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR add the option to use deno as udf language, it includes fetch, web and crypto API support.

CREATE FUNCTION digest( t string ) RETURNS bytea LANGUAGE javascript RUNTIME deno AS $$
    const subtle = crypto.subtle; 
    const key = await subtle.generateKey({
        name: 'HMAC',
        hash: 'SHA-256',
        length: 256,
        }, true, ['sign', 'verify']);
    
        const enc = new TextEncoder();
        const message = enc.encode(t);
    
        const result = await subtle.sign({
        name: 'HMAC',
        }, key, message);
        console.log('result', result);
        return result;
$$ ASYNC;

CREATE FUNCTION delay_response()
    RETURNS TABLE (x int) LANGUAGE javascript RUNTIME deno AS $$
                 const delayedResponses = {
                    delays: [50, 10, 15], 
                    wait(delay) {
                      return new Promise((resolve) => {
                        setTimeout(resolve, delay);
                      });
                    },
                  
                    async *[Symbol.asyncIterator]() {
                      for (const delay of this.delays) {
                        await this.wait(delay);
                        yield delay;
                      }
                    },
                  };
                return delayedResponses;
$$   SYNC;

CREATE FUNCTION fetch_api() RETURNS TABLE ( data struct< id int, name string, gender string >) LANGUAGE javascript RUNTIME deno AS $$
    const response = await fetch('https://rickandmortyapi.com/api/character');
    const resp = await response.json();
    for (const r of resp.results) {
        yield r;
    }
$$ ASYNC GENERATOR;

I added support for setting the function type when creating a function

type	function	declaration
`ASYNC`	A java script async function, to enable the usage of the await api	`async function <name> { <body> }`
`ASYNC GENERATOR`	A java script async generator function, to enable the usage of the await and yield api	`async function* <name> { <body> }`
`GENERATOR`	A java script generator function, to enable the usage of the yield api	`function* <name> { <body> }`
`SYNC`	A regular javascript function, if a promise is returned, will wait until is fulfilled to return the result	`function <name> { <body> }`

If not defined will use normal for UDFs and generator for table functions;

I put the deno language support under a feature flag, which is currently enabled by default. (Plan to remove before this PR gets merged).

It will require arrow-udf/arrow-udf#7 to get merged

I'd also like to use one of the table functions as a source (to receive data from websockets or sse). But it's not currently supported. What do you think would be a good way to do it?

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

xxchan

Thanks for your amazing work! Could you share the background and your motivation of the work? like why do you need it, what's its advantage over current js UDF, what's not achievable now?

Users may feel strange that there are both js UDF and deno UDF, if they are not very familiar with js runtimes. So it's important to figure out whether we want both of them, and if so, make clear the difference for users.

bakjos · 2024-03-15T18:10:15Z

@xxchan TLDR The main motivation is being able to use web apis (fetch, crypto) as part of the UDFS, the full conversation here https://risingwave-community.slack.com/archives/C02T3F7UYM6/p1709825596494219

xxchan · 2024-03-15T18:21:17Z

Got it. Sorry that I missed the Slack discussion. For such large feature, it would still be nicer if we open an issue to have larger visibility and more discussion before the PR. ❤️

bakjos · 2024-03-20T21:57:45Z

Also do you think the sintax CREATE [TABLE|SOURCE ] .. AS FUNCTION <function_name> (param1, param2) could work?

wangrunji0408 · 2024-03-21T04:37:45Z

I added support for setting a parameter when creating the function to define the function_type:

According to postgres' documentation, the SET param TO value is to override a specific system parameter during the function call. Using this syntax to define properties of the function itself seems like a misuse.

For simplicity, is it possible to just assume async function for scalar functions and async function* for table functions? It seems possible because I see that all functions will be converted to promises in arrow-udf-js-deno. However, if explicit async configuration can not be avoided, I'd prefer to introduce a new ASYNC keyword.

src/cmd_all/Cargo.toml

wangrunji0408 · 2024-03-21T03:58:39Z

src/connector/src/sink/encoder/json.rs

+        let schema = serde_json::from_str::<Value>(&schema).unwrap();
+        let ans = serde_json::from_str::<Value>(ans).unwrap();
        assert_eq!(schema, ans);


This seems irrelevant to the UDF feature?

It was due an update in the dependencies, the order of the string changed, so I changed it to use json instead of just plain string, because the order should not matter

This was because deno_core adds the feature preserve_order for serde_json, I patched the dependency to remove it, but I'm not sure if this is the right place to do so

risingwave/Cargo.toml

Lines 309 to 310 in cd2c0d4

# patch to remove preserve_order from serde_json

deno_core = { git = "https://github.com/bakjos/deno_core", rev = "70d1544" }

I'm okay with this patch. cc @xxchan any better idea on this feature interaction?

src/frontend/src/handler/create_function.rs

src/sqlparser/src/ast/mod.rs

wangrunji0408 · 2024-03-21T06:13:08Z

Please also add some end-to-end tests in e2e_test/udf/js_deno_udf.slt. You can use the system command in sqllogictest to start a mock server. For example:

risingwave/e2e_test/udf/retry_python.slt

Lines 1 to 2 in fb7fadb

    
           system ok 
        
           python3 e2e_test/udf/test.py &

bakjos · 2024-03-21T15:58:03Z

I added support for setting a parameter when creating the function to define the function_type:

According to postgres' documentation, the SET param TO value is to override a specific system parameter during the function call. Using this syntax to define properties of the function itself seems like a misuse.

For simplicity, is it possible to just assume async function for scalar functions and async function* for table functions? It seems possible because I see that all functions will be converted to promises in arrow-udf-js-deno. However, if explicit async configuration can not be avoided, I'd prefer to introduce a new ASYNC keyword.

I like the idea of introducing new keywords, the main issue is with AsyncIterators vs generators in the table function since it changes the signature of the function, so SYNC ASYNC GENERATOR and ASYNC GENERATOR are needed

src/meta/model_v2/migration/src/m20240408_142249_runtime.rs

wangrunji0408 · 2024-04-10T08:38:02Z

Seems the deno e2e test is still flaky. 👀

Caused by:
    query failed: connection closed
    [SQL] select * FROM call_sse();
    at e2e_test/udf/deno_udf.slt:233

…os/deno_udf

bakjos · 2024-04-10T11:19:46Z

Seems the deno e2e test is still flaky. 👀

Caused by:
    query failed: connection closed
    [SQL] select * FROM call_sse();
    at e2e_test/udf/deno_udf.slt:233

@wangrunji0408 This started to happen after I switched from rust to python to the mock server, I changed the protocol from HTTP 1 to HTTP 1.1 to see if fix it

…os/deno_udf

bakjos · 2024-04-10T15:09:40Z

@wangrunji0408 Updating the protocol didn't work, so I made some extra changes over arrow-udf/arrow-udf#12 and changed the tests to use a single mock server for both sse and rest, and it seems to have fixed the issue. I ran the test 100 times and got no crash/error. Also, merged the latest changes to run the tests twice, and it was also successful both times

xiangjinwu · 2024-04-11T04:36:24Z

hakari disabled by #16142 was added back by commit 9d1f3c8

https://github.com/risingwavelabs/risingwave/pull/15719/files/124b1bbf0f3bb288c1ea6760148d234348ca568c
https://github.com/risingwavelabs/risingwave/pull/15719/files/9d1f3c8df6f6799593d67879ed9322b4cadc63c4

cc @xxchan

This reverts commit a1b4b51.

xiangjinwu · 2024-04-11T05:10:41Z

Also unsure what's wrong here:
https://buildkite.com/risingwavelabs/pull-request/builds/47096#018ecb75-4727-4abc-9a4c-66cdafe0d97c

bakjos · 2024-04-11T10:47:38Z

@wangrunji0408 I created this PR #16263

bakjos requested a review from a team as a code owner March 15, 2024 16:36

wangrunji0408 requested review from TennyZhuang, wangrunji0408 and xxchan March 15, 2024 16:43

xxchan reviewed Mar 15, 2024

View reviewed changes

bakjos force-pushed the bakjos/deno_udf branch 2 times, most recently from 2e01bf8 to 61175b3 Compare March 18, 2024 14:34

bakjos requested a review from xxchan March 18, 2024 14:43

bakjos force-pushed the bakjos/deno_udf branch 2 times, most recently from e55f8c0 to 8fc2c6a Compare March 20, 2024 18:47

wangrunji0408 reviewed Mar 21, 2024

View reviewed changes

bakjos force-pushed the bakjos/deno_udf branch from 8fc2c6a to cc936ad Compare March 23, 2024 00:49

bakjos requested a review from wangrunji0408 March 23, 2024 00:52

bakjos force-pushed the bakjos/deno_udf branch 3 times, most recently from f0d4399 to a0c2285 Compare March 23, 2024 11:43

fuyufjh assigned wangrunji0408 Mar 25, 2024

bakjos force-pushed the bakjos/deno_udf branch from cd2c0d4 to d562baf Compare March 31, 2024 00:12

bakjos added 6 commits April 1, 2024 10:48

feat(udf): Add deno as UDF language

1698b08

chore: Update nightly rust version

23ce152

feat: Update arrow-udf-deno to latest commit

2f55dac

feat: Update ahash version

1c51eeb

feat: Update crc32c version

cf45366

feat: Rollback rust nightly toolchain version

7580838

wangrunji0408 added this pull request to the merge queue Apr 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2024

wangrunji0408 added this pull request to the merge queue Apr 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2024

wangrunji0408 added this pull request to the merge queue Apr 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2024

wangrunji0408 added this pull request to the merge queue Apr 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2024

yezizp2012 reviewed Apr 10, 2024

View reviewed changes

src/meta/model_v2/migration/src/m20240408_142249_runtime.rs Outdated Show resolved Hide resolved

bakjos added 3 commits April 10, 2024 06:12

feat: Update sse mock server protocol to 1.1

9d18e25

Merge branch 'main' of github.com:risingwavelabs/risingwave into bakj…

a49a4d9

…os/deno_udf

chore: Remove extra whitespaces

ce3226b

bakjos added 3 commits April 10, 2024 08:40

feat: Update deno-udf version

89b3d4c

Merge branch 'main' of github.com:risingwavelabs/risingwave into bakj…

0c2360b

…os/deno_udf

Merge branch 'main' into bakjos/deno_udf

3a9859e

wangrunji0408 added this pull request to the merge queue Apr 10, 2024

Merged via the queue into risingwavelabs:main with commit a1b4b51 Apr 10, 2024
31 checks passed

This was referenced Apr 11, 2024

chore(deps): Bump rustls from 0.22.3 to 0.23.4 #16240

Closed

chore(deps): Bump strum from 0.25.0 to 0.26.1 #16239

Closed

chore(deps): Bump the aws group with 2 updates #16237

Closed

xiangjinwu added a commit that referenced this pull request Apr 11, 2024

revert: "feat(udf): Add deno as UDF language (#15719)"

e205241

This reverts commit a1b4b51.

xiangjinwu mentioned this pull request Apr 11, 2024

revert: "feat(udf): Add deno as UDF language (#15719)" #16248

Merged

9 tasks

github-merge-queue bot pushed a commit that referenced this pull request Apr 11, 2024

revert: "feat(udf): Add deno as UDF language (#15719)" (#16248)

92ff468

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(udf): Add deno as UDF language #15719

feat(udf): Add deno as UDF language #15719

bakjos commented Mar 15, 2024 •

edited

Loading

xxchan left a comment

bakjos commented Mar 15, 2024 •

edited

Loading

xxchan commented Mar 15, 2024

bakjos commented Mar 20, 2024

wangrunji0408 commented Mar 21, 2024 •

edited

Loading

wangrunji0408 Mar 21, 2024

bakjos Mar 21, 2024 •

edited

Loading

bakjos Mar 23, 2024

wangrunji0408 Apr 2, 2024

wangrunji0408 commented Mar 21, 2024 •

edited

Loading

bakjos commented Mar 21, 2024 •

edited

Loading

wangrunji0408 commented Apr 10, 2024

bakjos commented Apr 10, 2024 •

edited

Loading

bakjos commented Apr 10, 2024 •

edited

Loading

xiangjinwu commented Apr 11, 2024

xiangjinwu commented Apr 11, 2024

bakjos commented Apr 11, 2024

	# patch to remove preserve_order from serde_json
	deno_core = { git = "https://github.com/bakjos/deno_core", rev = "70d1544" }

feat(udf): Add deno as UDF language #15719

feat(udf): Add deno as UDF language #15719

Conversation

bakjos commented Mar 15, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

xxchan left a comment

Choose a reason for hiding this comment

bakjos commented Mar 15, 2024 • edited Loading

xxchan commented Mar 15, 2024

bakjos commented Mar 20, 2024

wangrunji0408 commented Mar 21, 2024 • edited Loading

wangrunji0408 Mar 21, 2024

Choose a reason for hiding this comment

bakjos Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

bakjos Mar 23, 2024

Choose a reason for hiding this comment

wangrunji0408 Apr 2, 2024

Choose a reason for hiding this comment

wangrunji0408 commented Mar 21, 2024 • edited Loading

bakjos commented Mar 21, 2024 • edited Loading

wangrunji0408 commented Apr 10, 2024

bakjos commented Apr 10, 2024 • edited Loading

bakjos commented Apr 10, 2024 • edited Loading

xiangjinwu commented Apr 11, 2024

xiangjinwu commented Apr 11, 2024

bakjos commented Apr 11, 2024

bakjos commented Mar 15, 2024 •

edited

Loading

bakjos commented Mar 15, 2024 •

edited

Loading

wangrunji0408 commented Mar 21, 2024 •

edited

Loading

bakjos Mar 21, 2024 •

edited

Loading

wangrunji0408 commented Mar 21, 2024 •

edited

Loading

bakjos commented Mar 21, 2024 •

edited

Loading

bakjos commented Apr 10, 2024 •

edited

Loading

bakjos commented Apr 10, 2024 •

edited

Loading