Re: segfault in HEAD when too many nested functions call

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: segfault in HEAD when too many nested functions call
Date: 2017-07-21 11:40:23
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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().


Andres Freund

Attachment Content-Type Size
0001-Move-ExecProcNode-from-dispatch-to-callback-based-mo.patch text/x-patch 67.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alik Khilazhev 2017-07-21 11:51:00 Re: [WIP] Zipfian distribution in pgbench
Previous Message Andres Freund 2017-07-21 11:06:10 Re: pgsql: Add a Gather executor node.