Richard Guo <guofenglinux@gmail.com> 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
