On Thu, Jan 28, 2016 at 12:57:36PM +0900, Michael Paquier wrote:
> On Thu, Jan 28, 2016 at 12:40 PM, Noah Misch <noah@leadboat.com> wrote:
> + * fresh transaction. No part of core PostgreSQL functions that way,
> + * though it's a fair thing to want. Such code would wish the portal
> From the point of view of core code, this stands true, but, for my 2c,
> honestly, I think that is just going to annoy more people working on
> plugins and forks of Postgres. When working on Postgres-XC and
> developing stuff for the core code, I recall having been annoyed a
> couple of times by similar assert limitations
At first, I left out that assertion in case some extension code did the thing
I described, perhaps in a background worker. I then realized that
MarkPortalFailed() is the wrong thing for such code, which would want
treatment similar to this bit of PreCommit_Portals():
/* * Do not touch active portals --- this can only happen in the case of * a multi-transaction utility
command,such as VACUUM. * * Note however that any resource owner attached to such a portal is * still going
togo away, so don't leave a dangling pointer. */ if (portal->status == PORTAL_ACTIVE) {
portal->resowner= NULL; continue; }
If you can think of a case where the code would work okay despite its active
portal being marked as failed, that would be a good reason to omit the one
assertion. Otherwise, an assertion seems better than silently doing the
known-wrong thing.