The method you propose seems to be very specific to correlation variables, so at first I thought that the method should be in Correlate. I now agree that is belongs in RelNode/AbstractRelNode, because variables they are propagated through nodes of all types, even though they are mainly of interest to Correlate.
So, I would be OK having the method in AbstractRelNode as long as it is small (i.e. if there is complex code put it elsewhere such as RexUtil) and the method name and documentation clearly indicate that it is about correlation variables. "copy(List<RexNode>)" sounds general-purpose, which it isn't. Also, take care when copying/combining RelNode trees with variables. I don't know when it is safe to combine variables and when you need to define new ones. I wonder whether your method needs an extra parameter to describe the mapping of old:new variables. Julian > On Nov 23, 2014, at 7:05 AM, Vladimir Sitnikov <[email protected]> > wrote: > > Hi, > > I need a kind of visitor that will be able to rewrite part of relation > tree including rex expressions. > RexShuttle does a good job when rewriting Rex expressions alone (e.g. > calulations, casts, etc). > > RelShuttle allows to visit Rel tree (e.g. Project/Filter/etc), however > it does not allow to change expressions inside the tree. > > Here's my question: does anybody object if I add copy(List<RexNode> > newChildExps) to AbstractRelNode? > > The use-case is as follows: when Calcite creates correlated plans, it > might reference the same source relation multiple times under > different correlation variable names. > > For instance, here's the sample plan: Correlate(TableScan(A), > Filter(b.x=$cor1.x and b.y=$cor2.y, Scan(B))). > Latter in the processing, Calcite resolves those $cor1 and $cor2, and > I would like to rewrite the tree to make sure the same correlation > input is referenced by a single name (e.g. always as $cor1). > > Currently I use RelShuttle, however I have to manually match > LogicalFilter/LogicalProject/LogicalJoin and create some (different!) > boiler-plate logic to call the appropriate copy(...) method. > > -- > Vladimir Sitnikov
