1
0
mirror of https://github.com/postgres/postgres.git synced 2025-04-21 12:05:57 +03:00

Future-proof the recursion inside ExecShutdownNode().

The API contract for planstate_tree_walker() callbacks is that they
take a PlanState pointer and a context pointer.  Somebody figured
they could save a couple lines of code by ignoring that, and passing
ExecShutdownNode itself as the walker even though it has but one
argument.  Somewhat remarkably, we've gotten away with that so far.
However, it seems clear that the upcoming C2x standard means to
forbid such cases, and compilers that actively break such code
likely won't be far behind.  So spend the extra few lines of code
to do it honestly with a separate walker function.

In HEAD, we might as well go further and remove ExecShutdownNode's
useless return value.  I left that as-is in back branches though,
to forestall complaints about ABI breakage.

Back-patch, with the thought that this might become of practical
importance before our stable branches are all out of service.
It doesn't seem to be fixing any live bug on any currently known
platform, however.

Discussion: https://postgr.es/m/208054.1663534665@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2022-09-19 12:16:02 -04:00
parent 4fd1479494
commit c35ba141de
3 changed files with 11 additions and 4 deletions

View File

@ -1691,7 +1691,7 @@ ExecutePlan(EState *estate,
* point. * point.
*/ */
if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD)) if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
(void) ExecShutdownNode(planstate); ExecShutdownNode(planstate);
if (use_parallel_mode) if (use_parallel_mode)
ExitParallelMode(); ExitParallelMode();

View File

@ -121,6 +121,7 @@
static TupleTableSlot *ExecProcNodeFirst(PlanState *node); static TupleTableSlot *ExecProcNodeFirst(PlanState *node);
static TupleTableSlot *ExecProcNodeInstr(PlanState *node); static TupleTableSlot *ExecProcNodeInstr(PlanState *node);
static bool ExecShutdownNode_walker(PlanState *node, void *context);
/* ------------------------------------------------------------------------ /* ------------------------------------------------------------------------
@ -768,8 +769,14 @@ ExecEndNode(PlanState *node)
* Give execution nodes a chance to stop asynchronous resource consumption * Give execution nodes a chance to stop asynchronous resource consumption
* and release any resources still held. * and release any resources still held.
*/ */
bool void
ExecShutdownNode(PlanState *node) ExecShutdownNode(PlanState *node)
{
(void) ExecShutdownNode_walker(node, NULL);
}
static bool
ExecShutdownNode_walker(PlanState *node, void *context)
{ {
if (node == NULL) if (node == NULL)
return false; return false;
@ -789,7 +796,7 @@ ExecShutdownNode(PlanState *node)
if (node->instrument && node->instrument->running) if (node->instrument && node->instrument->running)
InstrStartNode(node->instrument); InstrStartNode(node->instrument);
planstate_tree_walker(node, ExecShutdownNode, NULL); planstate_tree_walker(node, ExecShutdownNode_walker, context);
switch (nodeTag(node)) switch (nodeTag(node))
{ {

View File

@ -239,7 +239,7 @@ extern PlanState *ExecInitNode(Plan *node, EState *estate, int eflags);
extern void ExecSetExecProcNode(PlanState *node, ExecProcNodeMtd function); extern void ExecSetExecProcNode(PlanState *node, ExecProcNodeMtd function);
extern Node *MultiExecProcNode(PlanState *node); extern Node *MultiExecProcNode(PlanState *node);
extern void ExecEndNode(PlanState *node); extern void ExecEndNode(PlanState *node);
extern bool ExecShutdownNode(PlanState *node); extern void ExecShutdownNode(PlanState *node);
extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node); extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node);