public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: Use outerPlanState macro instead of referring to leffttree
Date: Wed, 06 Jul 2022 10:48:21 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs4-yHp5-ejJ6hywNamiXJgv-6K4bnyFQqvS3GXu-XHoExw@mail.gmail.com>
References: <CAMbWs4-vYhh1xsa_veah4PUed2Xq=Ed_YH3=Mqt5A3Y=EgfCEg@mail.gmail.com>
	<[email protected]>
	<CAMbWs4-yHp5-ejJ6hywNamiXJgv-6K4bnyFQqvS3GXu-XHoExw@mail.gmail.com>

Richard Guo <[email protected]> writes:
> On Sat, Jul 2, 2022 at 5:32 AM Tom Lane <[email protected]> wrote:
>> Backing up a little bit, one thing not to like about the outerPlanState
>> and innerPlanState macros is that they lose all semblance of type
>> safety:

> Your concern makes sense. I think outerPlan and innerPlan macros share
> the same issue. Not sure if there is a way to do the type check.

Yeah, I don't know of one either.  It needn't hold up this patch.

>> Would it make sense to try to use the local-variable style everywhere?

> Do you mean the pattern like below?
>   outerPlanState(hashstate) = ExecInitNode(outerPlan(node), estate, eflags);
> It seems that this pattern is mostly used when initializing child nodes
> with ExecInitNode(), and most calls to ExecInitNode() are using this
> pattern as a convention. Not sure if it's better to change them to
> local-variable style.

That's probably fine, especially if it's a commonly used pattern.

Typically, if one applies outerPlan() or outerPlanState() to the
wrong pointer, the mistake will become obvious upon even minimal
testing.  My concern here is more about usages in edge cases that
perhaps escape testing, for instance in the arguments of an
elog() for some nearly-can't-happen case.

			regards, tom lane

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Use outerPlanState macro instead of referring to leffttree
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox