Re: Fwd: Core dump with nested CREATE TEMP TABLE

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fwd: Core dump with nested CREATE TEMP TABLE
Date: 2016-01-31 15:54:20
Message-ID: 20160131155420.GE3990895@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 attached v2 manage to
improve PostgreSQL, or is it neutral-or-harmful like v1?

On Sat, Jan 30, 2016 at 04:28:47PM -0500, Robert Haas wrote:
> On Sat, Jan 30, 2016 at 2:13 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Sat, Jan 30, 2016 at 07:37:45AM -0500, Robert Haas wrote:
> > > On Fri, Jan 29, 2016 at 11:19 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > > On Thu, Jan 28, 2016 at 11:12:15AM -0500, Robert Haas wrote:
> > > >> The code you quote emits a warning
> > > >> about a reasonably forseeable situation that can never be right, but
> > > >> there's no particular reason to think that MarkPortalFailed is the
> > > >> wrong thing to do here if that situation came up.
> > > >
> > > > I have two reasons to expect these MarkPortalFailed() calls will be the wrong
> > > > thing for hypothetical code reaching them. First, PortalRun() and peers reset
> > > > ActivePortal and PortalContext on error in addition to fixing portal status
> > > > with MarkPortalFailed(). If code forgets to update the status, there's a
> > > > substantial chance it forgot the other two. My patch added a comment
> > > > explaining the second reason:
> > > >
> > > > + /*
> > > > + * See similar code in AtSubAbort_Portals(). This would fire if code
> > > > + * orchestrating multiple top-level transactions within a portal, such
> > > > + * as VACUUM, caught errors and continued under the same portal with a
> > > > + * fresh transaction. No part of core PostgreSQL functions that way,
> > > > + * though it's a fair thing to want. Such code would wish the portal
> > > > + * to remain ACTIVE, as in PreCommit_Portals(); we don't cater to
> > > > + * that. Instead, like AtSubAbort_Portals(), interpret this as a bug.
> > > > + */
> > >
> > > You may be right, but then again Tom had a different opinion, even
> > > after seeing your patch, and he's no dummy.
> >
> > Eh? Tom last posted to this thread before I first posted a patch.
>
> http://www.postgresql.org/message-id/29758.1451780533@sss.pgh.pa.us
> seems to me to be a vote against the concept embodied by the patch.

That much is true. The order of postings is the opposite of what you stated,
and there's no mailing list evidence that anyone other than you has read my
explanation of why MarkPortalFailed() is wrong here. Alternately, I demand
the schematics for Tom's time machine.

Attachment Content-Type Size
AtSubAbort_Portals-ACTIVE-v2.patch text/plain 2.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-01-31 16:41:38 Re: Proposal: Trigonometric functions in degrees
Previous Message Konstantin Knizhnik 2016-01-31 15:44:05 Re: PATCH: index-only scans with partial indexes