westonpace commented on code in PR #14799:
URL: https://github.com/apache/arrow/pull/14799#discussion_r1040654410
##########
cpp/src/arrow/engine/substrait/options.h:
##########
@@ -65,16 +69,27 @@ using NamedTableProvider =
std::function<Result<compute::Declaration>(const
std::vector<std::string>&)>;
static NamedTableProvider kDefaultNamedTableProvider;
+class ARROW_ENGINE_EXPORT ExtensionProvider {
+ public:
+ static std::shared_ptr<ExtensionProvider> kDefaultExtensionProvider;
+ virtual ~ExtensionProvider() = default;
+ virtual Result<RelationInfo> MakeRel(std::vector<DeclarationInfo> inputs,
+ const google::protobuf::Any& rel,
Review Comment:
Unfortunately, I don't think we can use `google::protobuf::Any` here. This
puts protobuf in the public API space. I thought it wasn't a big deal but it
ended up meaning that people building this package have to also have the
protobuf headers which is something we want to avoid.
We could use `void*` but that's a little clunky. Perhaps we could have
something like a marker interface...
```
class ExtensionDetail {};
class ARROW_ENGINE_EXPORT ExtensionProvider {
...
virtual Result<RelationInfo> MakeRel(std::vector<DeclarationInfo> inputs,
const ExtensionDetail& detail, const ExtensionSet& ext_set) = 0;
```
Then, in `options_internal.h`:
```
class ExtensionDetailImpl : public ExtensionDetail {
public:
google::protobuf::Any rel;
};
class ARROW_ENGINE_EXPORT ExtensionProviderBase {
public:
Result<RelationInfo> MakeRel(std::vector<DeclarationInfo> inputs, const
ExtensionDetail& detail, const ExtensionSet& ext_set) override {
return DoMakeRel(std::move(inputs),
static_cast<ExtensionDetailImpl>(detail).rel, ext_set);
}
protected:
Result<RelationInfo> DoMakeRel(std::vector<DeclarationInfo> inputs, const
google::protobuf::Any& rel, const ExtensionSet& ext_set) = 0;
};
```
--
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]
