You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by JiaTao Tao <ta...@gmail.com> on 2020/11/22 09:23:37 UTC

[DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

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

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by JiaTao Tao <ta...@gmail.com>.
Hi Julian
Mainly I agree with less code is more readable, but over less is also hard
to read for the beginner.

Regards!

Aron Tao


Chunwei Lei <ch...@gmail.com> 于2020年11月25日周三 下午3:54写道:

> I have the same feeling. But I am +0 on this change considering:
> 1)  we can not forbid users to write such code.
> 2) there might be other codes that are not debug-friendly and we can not
> change them all.
>
>
> Best,
> Chunwei
>
>
> On Tue, Nov 24, 2020 at 10:59 AM JiaTao Tao <ta...@gmail.com> wrote:
>
> > 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 <ja...@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 <jh...@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 <zi...@gmail.com> wrote:
> > > > >
> > > > > +1, `step into` causes bad debug experience.
> > > > >
> > > > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > > > > ~~~~~~~~~~~~~~~~~~
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by JiaTao Tao <ta...@gmail.com>.
Hi Chunwei
We can not update all, but we can try our best to make them less.

Regards!

Aron Tao


Chunwei Lei <ch...@gmail.com> 于2020年11月25日周三 下午3:54写道:

> I have the same feeling. But I am +0 on this change considering:
> 1)  we can not forbid users to write such code.
> 2) there might be other codes that are not debug-friendly and we can not
> change them all.
>
>
> Best,
> Chunwei
>
>
> On Tue, Nov 24, 2020 at 10:59 AM JiaTao Tao <ta...@gmail.com> wrote:
>
> > 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 <ja...@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 <jh...@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 <zi...@gmail.com> wrote:
> > > > >
> > > > > +1, `step into` causes bad debug experience.
> > > > >
> > > > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > > > > ~~~~~~~~~~~~~~~~~~
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Chunwei Lei <ch...@gmail.com>.
I have the same feeling. But I am +0 on this change considering:
1)  we can not forbid users to write such code.
2) there might be other codes that are not debug-friendly and we can not
change them all.


Best,
Chunwei


On Tue, Nov 24, 2020 at 10:59 AM JiaTao Tao <ta...@gmail.com> wrote:

> 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 <ja...@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 <jh...@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 <zi...@gmail.com> wrote:
> > > >
> > > > +1, `step into` causes bad debug experience.
> > > >
> > > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > > > ~~~~~~~~~~~~~~~~~~
> > >
> > >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by JiaTao Tao <ta...@gmail.com>.
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 <ja...@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 <jh...@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 <zi...@gmail.com> wrote:
> > >
> > > +1, `step into` causes bad debug experience.
> > >
> > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > > ~~~~~~~~~~~~~~~~~~
> >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Vladimir Sitnikov <si...@gmail.com>.
Stamatis>I think you can add a watch expression on RelBuilder#peek[1]
method and

Good luck with EnumerableBatchNestedLoopJoinRule :-)

    call.transformTo(
        EnumerableBatchNestedLoopJoin.create(
            convert(join.getLeft(), join.getLeft().getTraitSet()
                .replace(EnumerableConvention.INSTANCE)),
            convert(right, right.getTraitSet()
                .replace(EnumerableConvention.INSTANCE)),
            join.getCondition(),
            requiredColumns.build(),
            correlationIds,
            join.getJoinType()));

What I also do is I use conditional breakpoints:
a) Add a breakpoint to RelOptRuleCall#transformTo, and add a condition to
confine the rule or the relation (e.g. when I know its ID)
b) Add if (...) { "break".hashCode(); }  and put a breakpoint inside. That
works much faster than a conditional breakpoint for hot code.


Vladimir

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Stamatis Zampetakis <za...@gmail.com>.
Hey Aron,

I think you can add a watch expression on RelBuilder#peek[1] method and
like that you don't need to step in or anything like that during debugging.

Best,
Stamatis

[1]
https://github.com/apache/calcite/blob/99251a51842483bc80688364195a159b740bd53f/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L347

On Mon, Nov 23, 2020 at 11:38 PM James Starr <ja...@gmail.com> wrote:

> 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 <jh...@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 <zi...@gmail.com> wrote:
> > >
> > > +1, `step into` causes bad debug experience.
> > >
> > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > > ~~~~~~~~~~~~~~~~~~
> >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by James Starr <ja...@gmail.com>.
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 <jh...@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 <zi...@gmail.com> wrote:
> >
> > +1, `step into` causes bad debug experience.
> >
> > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> > ~~~~~~~~~~~~~~~~~~
>
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Julian Hyde <jh...@gmail.com>.
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 <zi...@gmail.com> wrote:
> 
> +1, `step into` causes bad debug experience.
> 
> On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
> ~~~~~~~~~~~~~~~~~~


Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Albert <zi...@gmail.com>.
+1, `step into` causes bad debug experience.

On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <ta...@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 <da...@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 <ta...@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 <ta...@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
~~~~~~~~~~~~~~~~~~

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by JiaTao Tao <ta...@gmail.com>.
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 <da...@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 <ta...@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 <ta...@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
> > > >
> > >
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Haisheng Yuan <hy...@apache.org>.
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 <da...@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 <ta...@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 <ta...@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
> > >
> >
> 

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by Danny Chan <da...@apache.org>.
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 <ta...@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 <ta...@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
> >
>

Re: [DISCUSS] Does anybody think this is debuging unfriendly: "call.transformTo(relBuilder.build())"

Posted by JiaTao Tao <ta...@gmail.com>.
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 <ta...@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
>