rtpsw commented on code in PR #13375:
URL: https://github.com/apache/arrow/pull/13375#discussion_r907718160
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -315,6 +315,11 @@ struct ExtensionIdRegistryImpl : ExtensionIdRegistry {
return Status::OK();
}
+ Status RegisterFunction(std::string uri, std::string name,
Review Comment:
The `Id` variation was there before my changes here, and I suspect the idea
of using `util::string_view` instead of `std::string` was to avoid copying
twice - once in converting any passed `util::string_view` (but not
`std::string`) typed arguments to `std::string` and second inside the
registration code. So, I believe the `util::string_view` variation should be
the one that stays, and the code inside needs to convert everything to a (held)
`std::string`. Most likely not everything was converted this way in the
original code - I recall getting a SIGSEGV when calling from Python before my
changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
