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".
Yes. We may do the same trick for righttree.
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:
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.
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.
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?
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.