Skip to content
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(query): bump arrow-udf version to enhance types supported by JS UDFs #15529

Merged
merged 2 commits into from
May 16, 2024

Conversation

maxjustus
Copy link
Contributor

@maxjustus maxjustus commented May 15, 2024

I made some enhancements to the arrow-udf-js code to improve Databend support.
JS UDFs now support Variant, Timestamp, Date and Decimal types in arguments and return values.
This update also enables all intrinsics in rquickjs for better JS compatibility.

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

…UDFs

I made some enhancements to the arrow-udf-js code to improve Databend support.
JS UDFs now support Variant, Timestamp, Date and Decimal types in arguments and
return values.
This version also enables all intrinsics in rquickjs for better JS compatibility.
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label May 15, 2024
@@ -224,8 +224,8 @@ parquet = { version = "51", features = ["async"] }
parquet_rs = { package = "parquet", version = "51" }

# Crates from risingwavelabs
arrow-udf-js = { package = "arrow-udf-js", git = "https://github.com/datafuse-extras/arrow-udf", rev = "a8fdfdd" }
arrow-udf-wasm = { package = "arrow-udf-wasm", git = "https://github.com/datafuse-extras/arrow-udf", rev = "a8fdfdd" }
arrow-udf-js = { package = "arrow-udf-js", git = "https://github.com/datafuse-extras/arrow-udf", rev = "d0a21f0" }
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use the upstream commit now.

@sundy-li sundy-li requested a review from b41sh May 15, 2024 03:40
Copy link
Member

@b41sh b41sh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution @maxjustus

@BohuTANG
Copy link
Member

Unit failed:

error: failed to get `arrow-udf-js` as a dependency of package `databend-query v0.1.0 (/workspace/src/query/service)`

Caused by:
  failed to load source for dependency `arrow-udf-js`

Caused by:
  Unable to update https://github.com/datafuse-extras/arrow-udf?rev=d0a21f0#d0a21f0f

Caused by:
  revspec 'd0a21f0fde330a0e5f658a55b58e0405d8372844' not found; class=Reference (4); code=NotFound (-3)

https://github.com/datafuselabs/databend/actions/runs/9088408924/job/24977918495?pr=15529

@maxjustus
Copy link
Contributor Author

Ah whoops @BohuTANG - my eyes somehow glossed over the fact that you all use a fork of the main arrow UDF repo. I was using a local crate override while developing this so I didn't notice. It looks like your fork was recently updated to include my changes at that SHA so I suspect that this CI pipeline will pass if you just retry it. I tried to myself just now, but I don't think I have permissions.

Copy link
Contributor

At least one type of change must be checked in the PR description.
@maxjustus please update it 🙏.

@maxjustus
Copy link
Contributor Author

ok @BohuTANG looks like it's all green now!

@sundy-li sundy-li enabled auto-merge May 16, 2024 00:02
@BohuTANG BohuTANG disabled auto-merge May 16, 2024 01:11
@BohuTANG BohuTANG merged commit 600f3ea into databendlabs:main May 16, 2024
76 of 78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants