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: add more components to the wasm-pack compatible list #8843

Merged
merged 4 commits into from
Jan 13, 2024

Conversation

waynexia
Copy link
Member

Which issue does this PR close?

Related to #7633

Rationale for this change

Make more components to be wasm-unknown-unknown compatible.

About the background and motivation: I'm trying to continue my side project that builds a web playground based on wasm for datafuson https://github.com/waynexia/datafusion-playground

What changes are included in this PR?

Add supports for datafusion-physical-plan, datafusion-execution and datafusion.

datafusion-physical-plan and datafusion-execution doesn't contain any logical changes. It works after I remove some unnecessary feature gates or dependence.

Some methods and logic in datafusion are commented out. The affected file is datafusion/core/src/datasource/listing/url.rs, all about parsing URL. This is a limitation from our dependence url. After some shallow investigates, I guess it's caused by OsStr from std isn't available in arch wasm-unknown-unknown.

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions bot added core Core DataFusion crate substrait labels Jan 12, 2024
Comment on lines +40 to 41
[dev-dependencies]
tokio = "1.17"
Copy link
Member Author

Choose a reason for hiding this comment

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

I move this to dev-dependencies casually. But datafusion-substrait and datafusion-proto are not supported yet.

Signed-off-by: Ruihang Xia <[email protected]>
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks good to me -- thank you @waynexia


datafusion-common = { workspace = true }
datafusion-execution = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked and this addition means that the wasm ci job now checks these crates
https://github.com/apache/arrow-datafusion/blob/8353a2ca2fd1f0ed5fc764b7463dfbcaa033ceef/.github/workflows/rust.yml#L192-L207

Here is the example CI run from this PR showing the new crates building in wasm https://github.com/apache/arrow-datafusion/actions/runs/7504130188/job/20430447784?pr=8843#step:6:194

@waynexia waynexia merged commit 965f4bc into apache:main Jan 13, 2024
22 checks passed
@waynexia waynexia deleted the improve-wasm branch January 13, 2024 03:11
@waynexia
Copy link
Member Author

Thanks for your review @alamb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate substrait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants