ljishen commented on PR #14681:
URL: https://github.com/apache/arrow/pull/14681#issuecomment-1329991273

   > I have a few requests. This seems like a fairly clever way to produce 
named table relations.
   > 
   > The Substrait serializer works on declarations. There is no need for a 
declaration to have a node factory (or even have a corresponding exec node). 
Would it be simpler to do something like:
   > 
   > ```
   > /// Declare a named table, there is currently no way to consume this node
   > class NamedTableOptions {
   >   public:
   >     std::vector<std::string> names;
   >     std::shared_ptr<Schema> table_schema;
   > };
   > 
   > ...
   > 
   > Declaration named_tbl{"named_table", NamedTableOptions{{"table", "1"}, 
schema}};
   > 
   > ...
   > 
   > Result<compute::ExecNode*> MakeNamedTableNode(compute::ExecPlan* plan,
   >                                                
std::vector<compute::ExecNode*> inputs,
   >                                                const 
compute::ExecNodeOptions& options) {
   >   return Status::Invalid("The named table node is for serialization 
purposes only and can never be converted into an exec plan or executed");
   > }
   > ...
   > 
   > DCHECK_OK(registry->AddFactory("named_table", MakeNamedTableNode));
   > ```
   
   Yes, I believe this is a better way to solve this.


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