public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: [email protected]
Subject: Re: Use outerPlanState macro instead of referring to leffttree
Date: Fri, 01 Jul 2022 17:32:02 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs4-vYhh1xsa_veah4PUed2Xq=Ed_YH3=Mqt5A3Y=EgfCEg@mail.gmail.com>
References: <CAMbWs4-vYhh1xsa_veah4PUed2Xq=Ed_YH3=Mqt5A3Y=EgfCEg@mail.gmail.com>

Richard Guo <[email protected]> writes:
> In the executor code, we mix use outerPlanState macro and referring to
> leffttree. Commit 40f42d2a tried to keep the code consistent by
> replacing referring to lefftree with outerPlanState macro, but there are
> still some outliers. This patch tries to clean them up.

Seems generally reasonable, but what about righttree?  I find a few
of those too with "grep".

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:

#define innerPlanState(node)		(((PlanState *)(node))->righttree)
#define outerPlanState(node)		(((PlanState *)(node))->lefttree)

You can pass any pointer you want, and the compiler will not complain.
I wonder if there's any trick (even a gcc-only one) that could improve
on that.  In the absence of such a check, people might feel that
increasing our reliance on these macros isn't such a hot idea.

Now, the typical coding pattern you've used:

 ExecReScanHash(HashState *node)
 {
+	PlanState  *outerPlan = outerPlanState(node);

is probably reasonably secure against wrong-pointer slip-ups.  But
I'm less convinced about that for in-line usages in the midst of
a function, particularly in the common case that the function has
a variable pointing to its Plan node as well as PlanState node.
Would it make sense to try to use the local-variable style everywhere?

			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