rtpsw commented on code in PR #33909:
URL: https://github.com/apache/arrow/pull/33909#discussion_r1095040557


##########
cpp/src/arrow/engine/substrait/options.h:
##########
@@ -83,6 +84,14 @@ class ARROW_ENGINE_EXPORT ExtensionProvider {
 
 ARROW_ENGINE_EXPORT std::shared_ptr<ExtensionProvider> 
default_extension_provider();
 
+struct ARROW_ENGINE_EXPORT NamedTapNodeOptions : public 
compute::ExecNodeOptions {
+  NamedTapNodeOptions(const std::string& name, std::shared_ptr<Schema> schema)
+      : name(name), schema(std::move(schema)) {}
+
+  std::string name;
+  std::shared_ptr<Schema> schema;
+};

Review Comment:
   Taking it a step further, we could just add `NamedTapRel` without any 
callback code, because one would be able (with #33850 and without this PR) 
register a custom extension provider that would handle it in `MakeRel` (and 
forward what it doesn't handle to the original/default extension provider). One 
step more, we could add nothing and just let the extension interpret an `Any` 
message. This feels like stepping in the wrong direction, which aggressively 
prioritizes generality. I feel we should have some structure in Acero for this, 
at least so that structures would get reused instead of reinvented.



-- 
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]

Reply via email to