Re: [HACKERS] segfault in HEAD when too many nested functions call - Mailing list pgsql-hackers

From Andres Freund
Subject Re: [HACKERS] segfault in HEAD when too many nested functions call
Date
Msg-id 20170721114023.fj6cvr6og46yogei@alap3.anarazel.de
Whole thread Raw
In response to Re: [HACKERS] segfault in HEAD when too many nested functions call  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] segfault in HEAD when too many nested functions call
Re: [HACKERS] segfault in HEAD when too many nested functions call
List pgsql-hackers
On 2017-07-18 16:53:43 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Sokolov Yura
Date:
Subject: Re: [HACKERS] Small improvement to compactify_tuples
Next
From: Alik Khilazhev
Date:
Subject: Re: [HACKERS] [WIP] Zipfian distribution in pgbench