|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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().
|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.|