Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-31 Thread Noah Misch
On Fri, Jul 28, 2017 at 02:42:06PM -0400, Robert Haas wrote: > On Fri, Jul 28, 2017 at 12:29 PM, Noah Misch wrote: > > Your colleagues achieve compliance despite uncertainty; for inspiration, I > > recommend examining Alvaro's status updates as examples of this. The policy > > currently governs y

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-30 Thread Julien Rouhaud
On 31/07/2017 01:47, Andres Freund wrote: > Julien, could you quickly verify that everything's good for you now too? > I just checked on current HEAD (cc9f08b6b813e30789100b6b34110d8be1090ba0) and everything's good for me. Thanks! -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- S

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-30 Thread Andres Freund
Hi, On 2017-07-29 16:14:08 -0400, Tom Lane wrote: > Andres Freund writes: > > [ 0002-Move-ExecProcNode-from-dispatch-to-function-pointer-.patch ] > > Here's a reviewed version of this patch. Thanks! I pushed both now. > I added dummy ExecProcNodeMtd functions to the various node types that >

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-29 Thread Tom Lane
Andres Freund writes: > [ 0002-Move-ExecProcNode-from-dispatch-to-function-pointer-.patch ] Here's a reviewed version of this patch. I added dummy ExecProcNodeMtd functions to the various node types that lacked them because they expect to be called through MultiExecProcNode instead. In the old

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-29 Thread Andres Freund
On 2017-07-29 14:20:32 -0400, Tom Lane wrote: > Here's a reviewed version of this patch. Thanks! > * I think you put ExecScan's CFI in the wrong place; AFAICT yours > only covers its fast path. Sure - but the old path already has a CFI? And it has to be inside the loop, because well, the loop ;)

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-29 Thread Tom Lane
Andres Freund writes: > On 2017-07-26 16:28:38 -0400, Tom Lane wrote: >> It's certainly possible that there are long-running loops not involving >> any ExecProcNode recursion at all, but that would be a bug independent >> of this issue. The CFI in ExecProcNode itself can be replaced exactly >> ei

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-28 Thread Robert Haas
On Fri, Jul 28, 2017 at 12:29 PM, Noah Misch wrote: > Your colleagues achieve compliance despite uncertainty; for inspiration, I > recommend examining Alvaro's status updates as examples of this. The policy > currently governs your open items even if you disagree with it. I emphatically agree wi

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-28 Thread Tom Lane
Andres Freund writes: > Anyway, I'll commit it after another pass in ~1 week if it doesn't get a > review till then, but I assume it'll. FWIW, I intend to review it today, or tomorrow at the very latest. (Right now I'm buried in perl droppings.) regards, tom lane -- Se

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-28 Thread Andres Freund
On 2017-07-28 09:29:58 -0700, Noah Misch wrote: > On Thu, Jul 27, 2017 at 10:08:57PM -0700, Andres Freund wrote: > > For me that means the policy isn't quite right. It's not like I can > > force Tom to review the patch at a specific date. But the thread has > > been progressing steadily over the l

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-28 Thread Noah Misch
On Thu, Jul 27, 2017 at 10:08:57PM -0700, Andres Freund wrote: > On 2017-07-27 22:04:59 -0700, Noah Misch wrote: > > On Thu, Jul 27, 2017 at 09:49:18PM -0700, Andres Freund wrote: > > > On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > > > > On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrot

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-27 Thread Andres Freund
On 2017-07-27 22:04:59 -0700, Noah Misch wrote: > On Thu, Jul 27, 2017 at 09:49:18PM -0700, Andres Freund wrote: > > On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > > > On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > > > > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrot

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-27 Thread Noah Misch
On Thu, Jul 27, 2017 at 09:49:18PM -0700, Andres Freund wrote: > On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > > On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > > > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > > > > > > > On July 24, 2017 7:10:19 AM

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-27 Thread Andres Freund
On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch > > > wrote: > > > >On Tue, Jul 18, 2017 at 01:04:

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-27 Thread Noah Misch
On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch wrote: > > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > > >> Ok, I'll flesh out the patc

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-27 Thread Andres Freund
On 2017-07-26 16:28:38 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2017-07-26 15:03:37 -0400, Tom Lane wrote: > >> Hm, that seems kinda backwards to me; I was envisioning the checks > >> moving to the callees not the callers. I think it'd end up being > >> about the same number of occur

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-26 Thread Noah Misch
On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch wrote: > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > >> Ok, I'll flesh out the patch till Thursday. But I do think we're > >going > >> to have to do somet

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-26 Thread Tom Lane
Andres Freund writes: > On 2017-07-26 15:03:37 -0400, Tom Lane wrote: >> Hm, that seems kinda backwards to me; I was envisioning the checks >> moving to the callees not the callers. I think it'd end up being >> about the same number of occurrences of CHECK_FOR_INTERRUPTS(), >> and there would be

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-26 Thread Andres Freund
On 2017-07-26 15:03:37 -0400, Tom Lane wrote: > Andres Freund writes: > > I've moved the CHECK_FOR_INTERRUPTS() to the callsites. That > > unsurprisingly ends up being somewhat verbose, and there's a bunch of > > minor judgement calls where exactly to place them. While doing so I've > > also added

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-26 Thread Tom Lane
Andres Freund writes: > I've moved the CHECK_FOR_INTERRUPTS() to the callsites. That > unsurprisingly ends up being somewhat verbose, and there's a bunch of > minor judgement calls where exactly to place them. While doing so I've > also added a few extra ones. Did this in a separate patch to make

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-25 Thread Andres Freund
Hi, On 2017-07-24 13:27:58 -0400, Tom Lane wrote: > Andres Freund writes: > >> * 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 ver

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-24 Thread Andres Freund
On 2017-07-24 13:27:58 -0400, Tom Lane wrote: > Andres Freund writes: > >> 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_

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-24 Thread Tom Lane
Andres Freund 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.

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-24 Thread Andres Freund
Hi, 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 yo

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-24 Thread Andres Freund
On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch wrote: >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: >> Ok, I'll flesh out the patch till Thursday. But I do think we're >going >> to have to do something about the back branches, too. > >This PostgreSQL 10 open item is past du

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-23 Thread Noah Misch
On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > Ok, I'll flesh out the patch till Thursday. But I do think we're going > to have to do something about the back branches, too. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 h

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-21 Thread Tom Lane
Andres Freund writes: > On 2017-07-18 16:53:43 -0400, Tom Lane wrote: >> 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.

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-21 Thread Julien Rouhaud
On 21/07/2017 13:40, Andres Freund wrote: > 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 sta

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-21 Thread Andres Freund
On 2017-07-18 16:53:43 -0400, Tom Lane wrote: > Andres Freund 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 im

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Tom Lane
Andres Freund 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 et

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Tom Lane
Andres Freund writes: > On 2017-07-18 15:45:53 -0400, Tom Lane wrote: >> While I'm uncomfortable with making such a change post-beta2, I'm one >> heck of a lot *more* uncomfortable with shipping v10 with no stack depth >> checks in the executor mainline. Fleshing this out and putting it in >> v10

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Andres Freund
On 2017-07-18 15:45:53 -0400, Tom Lane wrote: > Andres Freund writes: > > Attached is a trivial patch implementing 1) and a proof-of-concept hack > > for 2). > > The comment you made previously, as well as the proposed commit message > for 0001, suggest that you've forgotten that pre-v10 execQual

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Tom Lane
Andres Freund writes: > Attached is a trivial patch implementing 1) and a proof-of-concept hack > for 2). The comment you made previously, as well as the proposed commit message for 0001, suggest that you've forgotten that pre-v10 execQual.c had several check_stack_depth() calls. Per its header

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Julien Rouhaud
On 18/07/2017 14:04, Andres Freund wrote: > On 2017-07-17 23:04:43 +0200, Julien Rouhaud wrote: >> Is it v11 material or is there any chance to make it in v10? > > I think it'd make sense to apply the first to v10 (and earlier), and the > second to v11. I think this isn't a terribly risky patch,

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-18 Thread Andres Freund
On 2017-07-17 23:04:43 +0200, Julien Rouhaud wrote: > On 17/07/2017 16:57, Andres Freund wrote: > > The latter obviously isn't ready, but might make clearer what I'm > > thinking about. > > It did for me, and FWIW I like this approach. Cool. > > If we were to go this route we'd have to probably

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-17 Thread Julien Rouhaud
On 17/07/2017 16:57, Andres Freund wrote: > The latter obviously isn't ready, but might make clearer what I'm > thinking about. It did for me, and FWIW I like this approach. > If we were to go this route we'd have to probably move > the callback assignment into the ExecInit* routines, and possibl

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-17 Thread Andres Freund
Hi, On 2017-07-17 00:26:29 -0700, Andres Freund wrote: > I'm less convinced of that, due to the overhead argument. I think > there's a couple ways around that however: > > 1) We could move ExecProcNode() to be callback based. The first time a >node is executed a "wrapper" function is called

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-17 Thread Andres Freund
Hi, On 2017-07-15 11:22:37 -0400, Tom Lane wrote: > The thread drifted off without any resolution, but clearly we need to > do something before 10.0 final. We need to do something, I'm less convinced that it's really v10 specific :/ > "SELECT so()" has a nontrivial targetlist so we end up runni

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-16 Thread Noah Misch
On Sat, Jul 15, 2017 at 11:22:37AM -0400, Tom Lane wrote: > Julien Rouhaud writes: > > Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you > > write queries which result in infinite recursion (or just too many > > nested function calls), execution ends with segfault instead of in

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-15 Thread Julien Rouhaud
On 15/07/2017 17:22, Tom Lane wrote: > Julien Rouhaud writes: >> Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you >> write queries which result in infinite recursion (or just too many >> nested function calls), execution ends with segfault instead of intended >> exhausted max_

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-15 Thread Tom Lane
I wrote: > I still think that we really need to add a check in ExecProcNode(). Actually ... to what extent could a check in ExecInitNode() substitute for that? Or do we need both? How about ExecEndNode() and ExecReScan()? You could argue that the latter two tree traversals are unlikely either t

Re: [HACKERS] segfault in HEAD when too many nested functions call

2017-07-15 Thread Tom Lane
Julien Rouhaud writes: > Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you > write queries which result in infinite recursion (or just too many > nested function calls), execution ends with segfault instead of intended > exhausted max_stack_depth: Yes. We discussed this befor

[HACKERS] segfault in HEAD when too many nested functions call

2017-07-14 Thread Julien Rouhaud
Hello, Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you write queries which result in infinite recursion (or just too many nested function calls), execution ends with segfault instead of intended exhausted max_stack_depth: Program received signal SIGSEGV, Segmentation fault.