Re: segfault in HEAD when too many nested functions call

On 2017-07-18 16:53:43 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > ... If we were to go this route we'd have to probably move
> > the callback assignment into the ExecInit* routines, and possibly
> > replace the switch in ExecInitNode() with another callback, assigned in
> > make_*, and implement callbacks for ExecShutdown, MultiExecProcNode etc.
>
> BTW, I don't see why you really need to mess with anything except
> ExecProcNode? Surely the other cases such as MultiExecProcNode are
> not called often enough to justify changing them away from the
> switch technology. Yeah, maybe it would be a bit cleaner if they
> all looked alike ... but if you're trying to make a patch that's
> as little invasive as possible for v10, I'd suggest converting just
> ExecProcNode to this style.

Yea, I was making that statement when not aiming for v10. Attached is a
patch that converts just ExecProcNode. The big change in comparison to
the earlier patch is that the assignment of the callback is now done in
the respective ExecInit* routines. As a consequence the ExecProcNode
callbacks now are static. I think we should do this fully in v11, I
removing dispatch routines like ExecInitNode() is a good idea, both
because it moves concerns more towards the nodes themselves - given the
growth of executor nodes that strikes me as a good idea.

I've also added stack depth checks to ExecEndNode(),
MultiExecProcNode(), ExecShutdownNode(), because it's not guaranteed
that ExecProcNode is called for every node...

I dislike having the miscadmin.h include in executor.h - but I don't
quite see a better alternative.

I want to run this through pgindent before merging, otherwise we'll
presumably end up with a lot of noise.

I still think we should backpatch at least the check_stack_depth() calls
in ExecInitNode(), ExecEndNode().

Greetings,

Andres Freund