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

[C++] Update uri_path parsing in FromProto #31799

Closed
asfimport opened this issue Apr 29, 2022 · 4 comments
Closed

[C++] Update uri_path parsing in FromProto #31799

asfimport opened this issue Apr 29, 2022 · 4 comments

Comments

@asfimport
Copy link

asfimport commented Apr 29, 2022

FromProto function in arrow/engine/substrait/relation_internal.cc parse uri_path with string_view utilities. However this should be done with Uri class from arrow/util/uri.h.

else if (util::string_view{path}.ends_with(".arrow")) {
  format = std::make_shared<dataset::IpcFileFormat>();
} else if (util::string_view{path}.ends_with(".feather")) {
  format = std::make_shared<dataset::IpcFileFormat>();

Reporter: Ariana Villegas / @ArianaVillegas
Assignee: Aldrin Montana / @drin

Related issues:

PRs and other links:

Note: This issue was originally created as ARROW-16424. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Weston Pace / @westonpace:
When writing the python Substrait consumer we disabled one of the tests on Windows due to this issue (it was described as ARROW-16392 which is a duplicate of this issue). When this is fixed we should make sure to re-enable the test.

@asfimport
Copy link
Author

Aldrin Montana / @drin:
I wanted to note some of the unit tests that were disabled due to ARROW-16392, which can be re-enabled when this is addressed:

@asfimport
Copy link
Author

Aldrin Montana / @drin:
I have assigned this issue to myself. I tried to extend PR#13390, but that became complicated after I rebased.

 

I have created PR#14071 for reference, but I will also try to coordinate with @vibhatha  for ARROW-16855.

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
Issue resolved by pull request 14071
#14071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant