You are right! Always used RexShuttle directly. To make it parallel, we could have named the new Rel class RelVisitor<T>, but RelVisitor is already taken.
On Thu, Dec 2, 2021 at 1:16 AM Ruben Q L <[email protected]> wrote: > @Walaa Eldin Moustafa <[email protected]>, for RexNodes there seems > to be already this "generic return type visitor pattern" via RexVisitor; in > fact, RexShuttle is just an implementation of RexVisitor. > > > > > On Thu, Dec 2, 2021 at 9:06 AM Walaa Eldin Moustafa <[email protected]> > wrote: > >> +1 while maintaining backward compatibility. Is there a plan for >> RexShuttle >> too? >> >> On Thu, Dec 2, 2021 at 1:03 AM Ruben Q L <[email protected]> wrote: >> >> > Sounds like a good idea, as long as it does not break backwards >> > compatibility. >> > >> > Regards, >> > Ruben >> > >> > On Thu, Dec 2, 2021 at 7:22 AM Jay Narale <[email protected]> >> wrote: >> > >> > > +1 re-writing to other types would be easier with this >> > > >> > > On Thu, Dec 2, 2021 at 16:18 Alessandro Solimando < >> > > [email protected]> wrote: >> > > >> > > > Hi Jacques, >> > > > I have faced the same issue recently and I think it's a good idea to >> > > have a >> > > > generic version of it and have RelShuttle be one of its concrete >> > > > implementations. >> > > > >> > > > Best regards, >> > > > Alessandro >> > > > >> > > > >> > > > Il Gio 2 Dic 2021, 06:02 Jacques Nadeau <[email protected]> ha >> > scritto: >> > > > >> > > > > Right now, RelNode's accept method and RelShuttle forces a user to >> > > return >> > > > > RelNodes. This makes tree traversal/conversion extra painful in >> main >> > > > > circumstances (you want to have more specific relnode classes >> > explicit, >> > > > you >> > > > > want to rewrite to non-relnodes, etc). I've worked around this for >> > many >> > > > > years but really think it should be resolved inside Calcite. I >> wanted >> > > to >> > > > > get some feedback on introducing a more generic pattern (that >> > > RelShuttle >> > > > > becomes a concrete variation of). If people are on board with the >> > > change, >> > > > > I'll pull it through the codebase. >> > > > > >> > > > > You can see the rough idea in this wip patch: >> > > > > https://github.com/apache/calcite/pull/2625 >> > > > > >> > > > > Basically, introduce a new parameterized visitor that people can >> use. >> > > > This >> > > > > doesn't change the existing behavior of RelShuttle (it's now just >> a >> > > > > specific version of this generic version) but it does allow other >> > > rewrite >> > > > > patterns. >> > > > > >> > > > > Thoughts? >> > > > > >> > > > >> > > -- >> > > Warm Regards, >> > > >> > > Jay Narale >> > > >> > >> >
