Hi James My point is not all intermediate variables, please don't expand the scope, you may not family with "relBuilder.build()", you can not run this method twice, it's not idempotent.
I'm usually using "option+click" to direct see the expression's value, it's so convenient, but RelBuilder#peek is a good way. Regards! Aron Tao James Starr <jamesst...@gmail.com> 于2020年11月24日周二 上午6:38写道： > I second readability over debug ability. If every function call is > decompressed to its own variable, readability can be compromised while make > it more annoy to debug because there are now so many > intermediate variables. Calcite compiles relatively quickly, it is not too > inconvenient to refactor a relevant portion of code to make debugging for a > particular task easier, then recompile, before refactoring for > readability/clarity at the end. > > James > > On Mon, Nov 23, 2020 at 11:49 AM Julian Hyde <jhyde.apa...@gmail.com> > wrote: > > > I am -0 on this change. > > > > Most code is read many more times than it is debugged. If you expand code > > so that it takes more screen area, the reader has to read more code in > > order to figure out what’s going on. > > > > Of course you can take that philosophy too far. If I have a multi-line > > expression, with large nested calls, it’s almost always worth creating > > intermediate variables for those calls. > > > > In the case of RelBuilder.build(), I happen to know that it is basically > > popping the top element off RelBuilder’s stack, so I would take a look at > > that stack rather than stepping in. > > > > Recent versions of Intellij allow you to choose which function you step > > into. If you are on the line > > > > call.transformTo(relBuilder.build()); > > > > and press f7 (step into), Intellij will ask you highlight ’transformTo’ > > and ‘build’ and let you choose one. > > > > By the way, some folks like to assign values to variables before they > > return them, e.g. > > > > final RelNode r = relBuilder.build(); > > return r; > > > > I am not fond of that pattern either. > > > > Julian > > > > > > > > > On Nov 23, 2020, at 6:32 AM, Albert <zinki...@gmail.com> wrote: > > > > > > +1, `step into` causes bad debug experience. > > > > > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <taojia...@gmail.com> > wrote: > > > > > >> I can create a JIRA and update the code, it's minor but I think it is > > good > > >> for us. > > >> > > >> > > >> Regards! > > >> > > >> Aron Tao > > >> > > >> > > >> Haisheng Yuan <hy...@apache.org> 于2020年11月23日周一 下午5:53写道： > > >> > > >>> Agree with Jiatao, I had the same experience and feeling. But it > mainly > > >>> depends on the rule creator's preference. > > >>> > > >>> On 2020/11/23 02:42:21, Danny Chan <danny0...@apache.org> wrote: > > >>>> I kind of agree, but it's more like a programming specification, we > > can > > >>>> tell people how to write codes but they may not follow those rules. > > >>>> > > >>>> JiaTao Tao <taojia...@gmail.com> 于2020年11月22日周日 下午5:27写道： > > >>>> > > >>>>> Why I don't want to debug into "transformTo": > > >>>>> > > >>>>> 1. It's a common method, if you directly stop here, every rule will > > >>> stop, > > >>>>> or you must stop the specific rule, then step into this method > call, > > >>> it's > > >>>>> one more step. > > >>>>> 2. There are many contexts in the rule, if you debug into > > >>> "transformTo", > > >>>>> you have to go back to see these. > > >>>>> > > >>>>> > > >>>>> Regards! > > >>>>> > > >>>>> Aron Tao > > >>>>> > > >>>>> > > >>>>> JiaTao Tao <taojia...@gmail.com> 于2020年11月22日周日 下午5:23写道： > > >>>>> > > >>>>>> Hi > > >>>>>> I've been developed Calcite full time for a quite long time, and I > > >>> ofter > > >>>>>> debug in the rule to see the transformations, but code like this > is > > >>> not > > >>>>>> debuging friendly in my opinion: > > >>> "call.transformTo(relBuilder.build())" > > >>>>>> > > >>>>>> I want to see the relBuilder.build()'s result, I have to debug > into > > >>> the > > >>>>>> "transformTo" method(you can not evaluate "relBuilder.build()" cuz > > >>> it's a > > >>>>>> stack), if we split this into two lines, we can just stop at the > > >> last > > >>>>> link: > > >>>>>> > > >>>>>> RelNode ret = relBuilder.build() > > >>>>>> call.transformTo(ret) > > >>>>>> > > >>>>>> It's not a big deal, but every time I occur this, it has poor > > >>>>> experience, hope > > >>>>>> to hear the community's opinion. > > >>>>>> > > >>>>>> Regards! > > >>>>>> > > >>>>>> Aron Tao > > >>>>>> > > >>>>> > > >>>> > > >>> > > >> > > > > > > > > > -- > > > ~~~~~~~~~~~~~~~ > > > no mistakes > > > ~~~~~~~~~~~~~~~~~~ > > > > >
