-
Notifications
You must be signed in to change notification settings - Fork 7
Add additional extension URI to Arrow Substrait Compiler #208
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
Conversation
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.
A few notes...I think the "just use the name with strsplit" plan is probably not going to work as simply as I had hoped.
Could you add an explicit test in test-pkg-arrow.R making sure that arrow_substrait_compiler(mtcars)$resolve_function("add", 1, 2)
(or something similarly simplified)` resolves the correct URI anchor? Similarly, I think an explicit test of actually evaluating a plan with a function from each URI would be appropriate (I believe there's already an explicit test for a plan with "add").
Co-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
.github/workflows/R-CMD-check.yaml
Outdated
@@ -94,7 +94,7 @@ jobs: | |||
|
|||
- name: Install DuckDB | |||
run: | | |||
R -e "remotes::install_github("duckdb/[email protected]", subdir ='tools/rpkg', build = FALSE)" | |||
R -e "remotes::install_github('duckdb/[email protected]', subdir ='tools/rpkg', build = FALSE)" |
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.
R -e "remotes::install_github('duckdb/duckdb@0.6.0', subdir ='tools/rpkg', build = FALSE)" | |
R -e "remotes::install_github('cran/duckdb@0.5.0', build = FALSE)" |
...maybe?
I just pushed one more try at getting DuckDB to work in CI. I get a clean CMD check from my end, but I do see:
When I run |
If that duckdb hack doesn't work, open a ticket and I'll take a look. It would also be nice to cache the Arrow build so that it doesn't have to rebuild for every commit and both of those could be the same ticket (or maybe DuckDB's release of 0.6.0 will solve that problem entirely). |
I've opened #211 and skipped those tests running if DuckDB isn't installed with Substrait. |
Fixes #202
See the unskipped function in this PR (which uses the comparison function "gt", which we hadn't previously implemented here) to see this working.