-
Notifications
You must be signed in to change notification settings - Fork 107
Map view types when converting arrow_expressions to vortex expressions #5765
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
base: develop
Are you sure you want to change the base?
Conversation
I, Paul Timmins <paul@iqmo.com>, hereby add my Signed-off-by to this commit: 8ae0c04 Signed-off-by: Paul Timmins <paul@iqmo.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
danking
left a comment
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.
This is great and we should merge! Can you follow up with a test that writes and reads through DuckDB a Vortex file containing strings? I think paultiq included one at one point in one of his GitHub issues.
|
@danking Do you want that test case in this PR or a separate? Happy to do either. |
|
Separate is fine! Sorry I referred to you in the third person! GitHub mobile UI had me turned around haha |
I, Paul Timmins <paul@iqmo.com>, hereby add my Signed-off-by to this commit: de97c5b Signed-off-by: Paul Timmins <paul@iqmo.com>
Fixes #5759
This PR maps string_views to strings and binary_views to binary so that
to_substraitwill no longer raise ArrowNotImplementedError when constructing the substrait.Vortex supports expressions over views and Arrow compute doesn't, but to_substrait raises ArrowNotImplementedError based on Arrow compute kernels... regardless of the backend.