Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sun, Jan 31, 2016 at 11:50 AM, Tom Lane wrote: > Noah Misch writes: >> On Sat, Jan 30, 2016 at 04:28:47PM -0500, Robert Haas wrote: >>> I think I've >>> pretty much said what I have to say about this; if nothing I wrote up >>> until now swayed you, it's

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Noah Misch writes: > On Sat, Jan 30, 2016 at 04:28:47PM -0500, Robert Haas wrote: >> I think I've >> pretty much said what I have to say about this; if nothing I wrote up >> until now swayed you, it's unlikely that anything else I say after >> this point will either. > Say I

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Jan 30, 2016 at 04:28:47PM -0500, Robert Haas wrote: > I think I've > pretty much said what I have to say about this; if nothing I wrote up > until now swayed you, it's unlikely that anything else I say after > this point will either. Say I drop the parts that change the binary. Does the

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Jan 30, 2016 at 2:13 PM, Noah Misch wrote: > You could offer that paragraph as an objection to almost all Assert(), elog(), > and automated tests. Why levy it against this patch? The valuable ways > assertions and tests supplement review are well-established. Sure,

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Jan 30, 2016 at 07:37:45AM -0500, Robert Haas wrote: > On Fri, Jan 29, 2016 at 11:19 PM, Noah Misch wrote: > > As you say, forbidding things makes friction in the event that someone comes > > along wanting to do the forbidden thing. Forbidding things also simplifies >

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Fri, Jan 29, 2016 at 11:19 PM, Noah Misch wrote: > As you say, forbidding things makes friction in the event that someone comes > along wanting to do the forbidden thing. Forbidding things also simplifies > the system, making it easier to verify. This decision should

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Thu, Jan 28, 2016 at 11:12:15AM -0500, Robert Haas wrote: > I'm honestly failing to understand why we should change anything at > all. I don't believe that doing something more severe than marking > the portal failed actually improves anything. I suppose if I had to > pick between what you

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Jan 27, 2016 at 11:47 PM, Noah Misch wrote: > On Wed, Jan 27, 2016 at 11:04:33PM -0500, Robert Haas wrote: >> +Assert(portal->status != PORTAL_ACTIVE); >> if (portal->status == PORTAL_ACTIVE) >>

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Thu, Jan 28, 2016 at 12:40 PM, Noah Misch wrote: > On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote: >> On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote: >> > Noah Misch writes: >> > > I am inclined to add an Assert(portal->status !=

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote: > On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote: > > Noah Misch writes: > > > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to > > > emphasize > > > that this is backup only.

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Jan 27, 2016 at 10:40 PM, Noah Misch wrote: > On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote: >> On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote: >> > Noah Misch writes: >> > > I am inclined to add an Assert(portal->status !=

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Jan 27, 2016 at 11:04:33PM -0500, Robert Haas wrote: > +Assert(portal->status != PORTAL_ACTIVE); > if (portal->status == PORTAL_ACTIVE) > MarkPortalFailed(portal); > > Now that just looks kooky to me. We assert that the portal isn't >

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Thu, Jan 28, 2016 at 12:57:36PM +0900, Michael Paquier wrote: > On Thu, Jan 28, 2016 at 12:40 PM, Noah Misch wrote: > + * fresh transaction. No part of core PostgreSQL functions that way, > + * though it's a fair thing to want. Such code would wish the

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Noah Misch writes: > On Thu, Sep 03, 2015 at 11:04:11PM -0400, Tom Lane wrote: >> *** AtSubAbort_Portals(SubTransactionId mySu >> --- 909,966 >> { >> Portal portal = hentry->portal; >> >> +/* Was it created in this subtransaction? */

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Jan 02, 2016 at 01:46:07PM -0500, Tom Lane wrote: > Noah Misch writes: > > On Thu, Sep 03, 2015 at 11:04:11PM -0400, Tom Lane wrote: > >> *** AtSubAbort_Portals(SubTransactionId mySu > > >> --- 909,966 > >> { > >> Portal portal =

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Noah Misch writes: > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to emphasize > that this is backup only. MarkPortalActive() callers remain responsible for > updating the status to something else before relinquishing control. No, I do not think that would

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote: > Noah Misch writes: > > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to emphasize > > that this is backup only. MarkPortalActive() callers remain responsible for > > updating the status to something

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Thu, Sep 03, 2015 at 11:04:11PM -0400, Tom Lane wrote: > *** AtSubAbort_Portals(SubTransactionId mySu > --- 909,966 > { > Portal portal = hentry->portal; > > + /* Was it created in this subtransaction? */ > if

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/4/15 12:41 PM, Tom Lane wrote: I wrote: After review of all the callers of RelationClearRelation, it seems like most of them already have sufficient guards to prevent triggering of the Assert. The ones that lack such tests are AtEOXact_cleanup and AtEOSubXact_cleanup. So what I'm now

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Sep 5, 2015 at 3:41 PM, Jim Nasby wrote: > I was about to test this and was verifying that the crash still worked > when I noticed this in the logs (9.4.1, not HEAD). Not sure if there's any > realion here or not... > > WARNING: relation "pg_proc" page 121 is

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > On Sat, Sep 5, 2015 at 3:41 PM, Jim Nasby wrote: >> I was about to test this and was verifying that the crash still worked >> when I noticed this in the logs (9.4.1, not HEAD). Not sure if there's any >> realion here

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > On Fri, Sep 4, 2015 at 12:04 PM, Tom Lane wrote: >> Attached is an updated patch with comments to this effect and some >> minor other code cleanup (mainly, not assuming that CurrentResourceOwner >> is the right thing to use

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Fri, Sep 4, 2015 at 10:52 PM, Tom Lane wrote: > Also, I am very strongly tempted to convert the original problem-causing > Assert in relcache.c into a regular runtime test-and-elog. If we're wrong > about having sealed off this hole completely, I'd much rather see the >

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

I wrote: > After review of all the callers of RelationClearRelation, it seems like > most of them already have sufficient guards to prevent triggering of the > Assert. The ones that lack such tests are AtEOXact_cleanup and > AtEOSubXact_cleanup. So what I'm now thinking is that those should do >

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > On Fri, Sep 4, 2015 at 10:52 PM, Tom Lane wrote: >> Also, I am very strongly tempted to convert the original problem-causing >> Assert in relcache.c into a regular runtime test-and-elog. If we're wrong >> about having

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

I wrote: >> After review of all the callers of RelationClearRelation, it seems like >> most of them already have sufficient guards to prevent triggering of the >> Assert. The ones that lack such tests are AtEOXact_cleanup and >> AtEOSubXact_cleanup. So what I'm now thinking is that those should

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Thu, Sep 3, 2015 at 1:46 AM, Jim Nasby wrote: > The double set of exceptions seems to be critical; if instead of calling > tf.get(invoice) (which recursively does tf.get(customer)) I define the > cursor to call tf.get(customer) directly there's no assert. Finally I

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > Finally I have been able to crack down the problem, and it can be > reproduced with the following test case for example: Hm. It looks like the problem is that we're executing the function within the Portal created for cursor "h", and that

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

I wrote: > I'm inclined to think that the only real fix for this type of problem > is that at subtransaction abort, we have to prevent further execution > of any Portals that have executed at all within the subxact (ie force > them into PORTAL_FAILED state and clean out their resources, see >

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Fri, Sep 4, 2015 at 6:51 AM, Tom Lane wrote: > I experimented with the attached patch. It seems to work to stop the > crash Michael exhibited (I've not tried to duplicate Jim's original > example, though). However, it also causes a regression test failure, > because

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > On Fri, Sep 4, 2015 at 6:51 AM, Tom Lane wrote: >> Ideas? > Yes. This diff on top of your patch: > @@ -922,8 +922,7 @@ AtSubAbort_Portals(SubTransactionId mySubid, > * must be forced into

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Fri, Sep 4, 2015 at 12:04 PM, Tom Lane wrote: > I wrote: > > Hmm. I am not feeling especially comfortable about this: it's not clear > > that there's anything preventing a suspended portal from containing a > > dangerous reference. However, a quick trial did not show

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Fri, Sep 4, 2015 at 9:57 AM, Tom Lane wrote: > On reflection I think that the tracking of activeSubid in my patch is > probably overkill if we're attacking it this way. We can just have > AtSubAbort_Portals fail any ACTIVE portal regardless of subxact level, > which is

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier writes: > On Fri, Sep 4, 2015 at 9:57 AM, Tom Lane wrote: >> On reflection I think that the tracking of activeSubid in my patch is >> probably overkill if we're attacking it this way. We can just have >> AtSubAbort_Portals fail any

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

I wrote: > Hmm. I am not feeling especially comfortable about this: it's not clear > that there's anything preventing a suspended portal from containing a > dangerous reference. However, a quick trial did not show that it was > possible to break it --- in the cases I tried, we detected that a

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Sep 2, 2015 at 6:21 AM, Michael Paquier wrote: > Yes, that's what I have been looking at actually by having some markers in > relcache.c. The reference count of this relation get incremented here: So, I have been playing more with this code, and as mentioned by Andres this test case goes

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/1/15 11:59 PM, Michael Paquier wrote: On Wed, Sep 2, 2015 at 12:59 PM, Jim Nasby wrote: On 9/1/15 8:42 PM, Michael Paquier wrote: The crash is triggered by having an exception raised in this particular call stack. Since there's no syntax error in master/0.2.1, there's no assert failure

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Sep 2, 2015 at 6:13 AM, Jim Nasby wrote: > On 9/1/15 11:59 PM, Michael Paquier wrote: >> >> On Wed, Sep 2, 2015 at 12:59 PM, Jim Nasby wrote: >>> >>> On 9/1/15 8:42 PM, Michael Paquier wrote: >>> The crash is triggered by having an exception raised in this

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/2/15 2:56 PM, Jim Nasby wrote: On 9/2/15 2:17 PM, Alvaro Herrera wrote: Michael Paquier wrote: I haven't written yet a test case but I think that we could reproduce that simply by having a relation referenced in the exception block of a first function, calling a second function that

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/2/15 2:17 PM, Alvaro Herrera wrote: Michael Paquier wrote: I haven't written yet a test case but I think that we could reproduce that simply by having a relation referenced in the exception block of a first function, calling a second function that itself raises an exception, causing the

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier wrote: > I haven't written yet a test case but I think that we could reproduce > that simply by having a relation referenced in the exception block of > a first function, calling a second function that itself raises an > exception, causing the referencing error. Hm, so function 2

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sun, Aug 30, 2015 at 1:06 AM, Jim Nasby wrote: > Steps to reproduce: > Download https://github.com/BlueTreble/test_factory/archive/crash.zip > Unzip, cd into directory > pgxn install pgtap (or just make test) FWIW, make test fails: ! ERROR: 42703: column "c_data_table_name" does not exist !

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Sep 2, 2015 at 9:39 AM, Michael Paquier wrote: > On Wed, Sep 2, 2015 at 7:23 AM, Jim Nasby wrote: >> Well nuts, pretty sure that means the error isn't reproducing for you. :/ Do >> you maybe have unusual config options or postgresql.conf options? > > Yep. And I

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Sep 2, 2015 at 7:23 AM, Jim Nasby wrote: > Well nuts, pretty sure that means the error isn't reproducing for you. :/ Do > you maybe have unusual config options or postgresql.conf options? Yep. And I have found one reason why it was not working, I have been using --extra-version with a

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/1/15 1:08 AM, Michael Paquier wrote: On Sun, Aug 30, 2015 at 1:06 AM, Jim Nasby wrote: Steps to reproduce: Download https://github.com/BlueTreble/test_factory/archive/crash.zip Unzip, cd into directory pgxn install pgtap (or just make test) FWIW, make test fails: ! ERROR: 42703: column

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/1/15 8:42 PM, Michael Paquier wrote: On Wed, Sep 2, 2015 at 9:39 AM, Michael Paquier wrote: On Wed, Sep 2, 2015 at 7:23 AM, Jim Nasby wrote: Well nuts, pretty sure that means the error isn't reproducing for you. :/ Do you maybe have unusual config options or

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 9/1/15 8:42 PM, Michael Paquier wrote: test_factory is a jungle to me. Perhaps you could just extract a self-contained test case? It does not matter if the file is long as long as the problem can be easily reproduced. Sorry, more info on what's happening here. The idea behind test_factory

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Wed, Sep 2, 2015 at 12:59 PM, Jim Nasby wrote: > On 9/1/15 8:42 PM, Michael Paquier wrote: > The crash is triggered by having an exception raised in this particular call > stack. Since there's no syntax error in master/0.2.1, there's no assert > failure either. Were you running asserts? I

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier michael.paqu...@gmail.com writes: Ah, OK, you meant this file... Yes I was able to receive it as well in your original email. I'll try to investigate further later, but Tom may beat me first. He usually does. Jim had indicated the bug wasn't reproducible on the strength of that

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 8/29/15 8:04 AM, Tom Lane wrote: Michael Paquier michael.paqu...@gmail.com writes: Ah, OK, you meant this file... Yes I was able to receive it as well in your original email. I'll try to investigate further later, but Tom may beat me first. He usually does. Jim had indicated the bug wasn't

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Hi, On 2015-08-29 11:06:05 -0500, Jim Nasby wrote: Stack trace below. Relevant assert: What exact revision is this on? Either there's some aggressive inlining going on, or these lines don't entirely match up with HEAD. That's compiled with optimization, isn't it? Could you compile with -O0?

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 8/29/15 11:31 AM, Andres Freund wrote: Hi, On 2015-08-29 11:06:05 -0500, Jim Nasby wrote: Stack trace below. Relevant assert: What exact revision is this on? Either there's some aggressive inlining going on, or these lines don't entirely match up with HEAD. Oops, that was 9.4.1. Trace

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On 8/28/15 8:39 PM, Tom Lane wrote: Michael Paquier michael.paqu...@gmail.com writes: On Sat, Aug 29, 2015 at 5:02 AM, Jim Nasby jim.na...@bluetreble.com wrote: Looks like a 98k file won't get through the list... Is it compressed? Note that we have sometimes larger patches than that, but

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

Michael Paquier michael.paqu...@gmail.com writes: On Sat, Aug 29, 2015 at 5:02 AM, Jim Nasby jim.na...@bluetreble.com wrote: Looks like a 98k file won't get through the list... Is it compressed? Note that we have sometimes larger patches than that, but perhaps those had special permissions by

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Aug 29, 2015 at 11:18 AM, Jim Nasby jim.na...@bluetreble.com wrote: On 8/28/15 8:39 PM, Tom Lane wrote: Michael Paquier michael.paqu...@gmail.com writes: On Sat, Aug 29, 2015 at 5:02 AM, Jim Nasby jim.na...@bluetreble.com wrote: Looks like a 98k file won't get through the list...

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

On Sat, Aug 29, 2015 at 5:02 AM, Jim Nasby jim.na...@bluetreble.com wrote: Looks like a 98k file won't get through the list... Is it compressed? Note that we have sometimes larger patches than that, but perhaps those had special permissions by the admins of this list. -- Michael