Skip to content

Conversation

@Sl1mb0
Copy link
Contributor

@Sl1mb0 Sl1mb0 commented Nov 11, 2025

Helps #31

This adds a AsyncScalerUDFImpl implementation for WasmScalarUDF & replaces uses of invoke_with_args with invoke_async_with_args.

  • I've read the contributing section of the project CONTRIBUTING.md.
  • Signed CLA (if not already signed).

@Sl1mb0 Sl1mb0 requested a review from crepererum November 11, 2025 04:24
host/src/lib.rs Outdated
let columnar_value: ColumnarValue = return_type.try_into()?;
match columnar_value {
ColumnarValue::Array(v) => Ok(v),
ColumnarValue::Scalar(v) => v.to_array_of_size(1),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output row count should be the same as the input. The args (IIRC) should have the respective row in them, so you can use that.

@Sl1mb0 Sl1mb0 requested a review from crepererum November 12, 2025 17:44
Copy link
Collaborator

@crepererum crepererum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nitpick. Feel free to merge after addressing this since this PR is rather conflict-heavy.


assert!(result.is_err());
let error = result.unwrap_err();
assert!(error.to_string().contains("synchronous invocation of WasmScalarUdf is not supported, use invoke_async_with_args instead"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep this in line with the remaining tests, I think this should use insta::assert_snapshot!(. This makes it easier to change the message and improves the assertion in case they actual and expected value mismatch.

@Sl1mb0 Sl1mb0 enabled auto-merge November 12, 2025 22:22
@Sl1mb0 Sl1mb0 added this pull request to the merge queue Nov 12, 2025
Merged via the queue into main with commit 53a7944 Nov 12, 2025
1 check passed
@Sl1mb0 Sl1mb0 deleted the tm/invoke-async branch November 12, 2025 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants