westonpace commented on code in PR #15253:
URL: https://github.com/apache/arrow/pull/15253#discussion_r1067558124
##########
cpp/src/arrow/compute/exec/exec_plan.h:
##########
@@ -121,18 +119,20 @@ class ARROW_EXPORT ExecNode {
virtual const char* kind_name() const = 0;
- // The number of inputs/outputs expected by this node
+ // The number of inputs expected by this node
int num_inputs() const { return static_cast<int>(inputs_.size()); }
- int num_outputs() const { return num_outputs_; }
/// This node's predecessors in the exec plan
const NodeVector& inputs() const { return inputs_; }
+ /// True if the plan has no output schema (is a sink)
+ bool is_sink() const { return !output_schema_; }
Review Comment:
I only need `is_sink` for validation purposes at the moment. If a node is a
sink we validate that `output_` is null (e.g. we don't let you try and use it
as an input during plan creation). So there are options here:
* We can get rid of the validation check and leave it to the individual
nodes (I think I may have already added this validation to `SinkNode`), this
would probably be my preference.
* We can pass in some `is_sink` bool to the `ExecNode` constructor (I'd
rather not do this as it is tedious but it is essentially what we had before)
I'll make sure I can implement the first option and, if so, get rid of this
concept.
That being said, I am a bit curious why this is important. I think it is
valid to say that a sink node doesn't have an output schema. We don't rely on
it having an output schema anywhere (or we shouldn't) that I know of.
--
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]
