Regarding HelpRelVertex::getInputs()

[Hari Sivarama Subramaniyan <hs...@hortonworks.com>]

?I was planning to raise a jira for Calcite but wanted to confirm if this is a bug in calcite HepRelVertex.

1. Is HepRelVertex.getInputs() suppose to return currentRel.getInputs() or empty list (as returned by AbstractRelNode).

2. Also I believe that AbstractRelNode.getInputs() should be made abstract method and not return an empty list as this is misleading and an extended class can accidentally skip the implementation.

Thanks
Hari?

Re: Regarding HelpRelVertex::getInputs()

[Julian Hyde <jh...@apache.org>]

If you're calling getInputs() on a HepRelVertex you're probably doing
the wrong thing. For instance, if the currentRel is a Join you
probably want to see the two inputs but you probably also want to know
that you are dealing with a Join and not, say, a Union.

So, it doesn't really matter what it returns. Maybe it should throw
UnsupportedOperationException. The safest thing is for it to do the
same as RelSubset (which has a similar purpose) and return the empty
list.

Regarding whether AbstractRelNode.getInputs should be abstract. Yes,
possibly. But it would be a breaking change, so we couldn't possibly
make it until 2.0.

I also note that there are only five overrides of getInputs. In other
words, most implementations of RelNode happily use one of the base
classes (BiRel, SingleRel, SetOp). And in fact most will derive from
one of the core relational operators (Filter, Project etc.) which are
themselves sub-classes of these base classes. Making getInputs
abstract would not make it significantly easier to write one's own
RelNode implementation.

Julian


On Mon, Feb 1, 2016 at 6:40 PM, Hari Sivarama Subramaniyan
<hs...@hortonworks.com> wrote:
> ?I was planning to raise a jira for Calcite but wanted to confirm if this is a bug in calcite HepRelVertex.
>
> 1. Is HepRelVertex.getInputs() suppose to return currentRel.getInputs() or empty list (as returned by AbstractRelNode).
>
> 2. Also I believe that AbstractRelNode.getInputs() should be made abstract method and not return an empty list as this is misleading and an extended class can accidentally skip the implementation.
>
> Thanks
> Hari?
>