Thanks again for the answer, we will work with that. One comment from my side: this lack of the contract is not ideal. I could imagine `accept(RexShuttle)` being used in the future for variety of things. For example if one day Calcite starts using `accept(RexShuttle)` for gathering used field references for column pruning, our code (that uses Calcite) can brake silently.
Piotrek > On 4 Feb 2019, at 20:04, Julian Hyde <[email protected]> wrote: > > RexShuttle does not have a watertight specification. It does what it needs to > do for the cases that use it. Generally, if the sub-elements of a RelNode are > sub-types of RexNode, RexShuttle processes them, otherwise it ignores them. > But pragmatically, as long as what you do doesn’t break the tests, it’s OK. > >> On Feb 4, 2019, at 1:05 AM, Piotr Nowojski <[email protected]> wrote: >> >> Hi Stamatis, >> >> Thank you for the answer! I have some questions to clarify that. >> >> 1. What if my new node doesn’t strictly have a `RexNode`, but it’s referring >> to the inputs indirectly, like via `String fieldName` instead of using >> RexInputRef? >> 2. Arguably this is the similar thing what `Aggregate` node is doing. Both >> `List<ImmutableBitSet> groupSets` and `List<AggregateCall> aggCalls` (in >> `Aggregate`) are referring to the operator/node inputs using indexes. >> >> Piotrek >> >>> On 2 Feb 2019, at 15:37, Stamatis Zampetakis <[email protected]> wrote: >>> >>> Hi Piotr, >>> >>> Aggregate is not doing anything in #accept(RexShuttle) since it does not >>> contain row expressions (RexNode). If the node you introduce uses row >>> expressions then it makes sense to apply the shuttle to every expression. >>> >>> Best, >>> Stamatis >>> >>> Στις Πέμ, 31 Ιαν 2019 στις 5:39 μ.μ., ο/η Piotr Nowojski < >>> [email protected]> έγραψε: >>> >>>> Hi! >>>> >>>> We are adding a new custom RelNode, that behaves a little bit like >>>> Aggregate node and we are wondering how we should implement >>>> >>>> RelNode#accept(org.apache.calcite.rex.RexShuttle) >>>> >>>> Our node has a similar feature as Aggregate node, that it’s keying by the >>>> incoming data. At first I thought that this #accept() method should be >>>> implemented by applying the RexShuttle to all of the expressions that node >>>> is using internally (in our case key fields “expressions”/“references"), >>>> like it’s done in Join, Sort, Filter, …. But then I noticed that this >>>> method is not implemented for Aggregate node. So can we leave this method >>>> with the default implementation `return this;` as Aggregate node does it? >>>> >>>> Thanks, Piotr Nowojski >> >
