From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Julien Rouhaud <julien.rouhaud@dalibo.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: segfault in HEAD when too many nested functions call
Date: Mon, 24 Jul 2017 13:27:58 -0400
Message-ID: <32759.1500917278@sss.pgh.pa.us> (raw)
In-Reply-To: <20170724161954.gbqswhukk7c4lygt@alap3.anarazel.de>
References: <b0af9eaa-130c-60d0-9e4e-7a135b1e0c76@dalibo.com>
<12302.1500132157@sss.pgh.pa.us>
<20170717072629.dqykbsot2irdob7c@alap3.anarazel.de>
<20170717145710.cm32m4yn555hsoxd@alap3.anarazel.de>
<6687.1500411223@sss.pgh.pa.us>
<20170721114023.fj6cvr6og46yogei@alap3.anarazel.de>
<19899.1500682674@sss.pgh.pa.us>
<20170724161954.gbqswhukk7c4lygt@alap3.anarazel.de>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Andres Freund <andres@anarazel.de> writes:
> On 2017-07-21 20:17:54 -0400, Tom Lane wrote:
>>> I dislike having the miscadmin.h include in executor.h - but I don't
>>> quite see a better alternative.
>> I seriously, seriously, seriously dislike that. You practically might as
>> well put miscadmin.h into postgres.h. Instead, what do you think of
>> requiring the individual ExecProcNode functions to perform
>> CHECK_FOR_INTERRUPTS? Since they're already responsible for doing that
>> if they have any long-running internal loops, this doesn't seem like a
>> modularity violation. It is a risk for bugs-of-omission, sure, but so
>> are a lot of other things that the per-node code has to do.
> That'd work. Another alternative would be to move the inline definition
> of ExecProcNode() (and probably a bunch of other related functions) into
> a more internals oriented header. It seems likely that we're going to
> add more inline functions to the executor, and that'd reduce the
> coupling of external and internal users a bit.
Well, it still ends up that the callers of ExecProcNode need to include
miscadmin.h, whereas if we move it into the per-node functions, then the
per-node files need to include miscadmin.h. I think the latter is better
because those files may need to have other CHECK_FOR_INTERRUPTS calls
anyway. It's less clear from a modularity standpoint that executor
callers should need miscadmin.h. (Or in short, I'm not really okay
with *any* header file including miscadmin.h.)
>> * I think the comments need more work. Am willing to make a pass over
>> that if you want.
> That'd be good, but let's wait till we have something more final.
Agreed, I'll wait till you produce another version.
>> * Can we redefine the ExecCustomScan function pointer as type
>> ExecProcNodeCB, eliminating the unsightly cast in nodeCustom.c?
> That'd change an "extension API", which is why I skipped it at this
> point of the release cycle. It's not like we didn't have this type of
> cast all over before. Ok, with changing it, but that's where I came
> down.
Is this patch really not changing anything else that a custom-scan
extension would touch? If not, I'm okay with postponing this bit
of cleanup to v11.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, julien.rouhaud@dalibo.com
Subject: Re: segfault in HEAD when too many nested functions call
In-Reply-To: <32759.1500917278@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox