Richard Guo <guofengli...@gmail.com> writes:
> On Sat, Jul 2, 2022 at 5:32 AM Tom Lane <t...@sss.pgh.pa.us> 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
