Re: 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
> active, but then cater to the possibility that it might be anyway?

Right.

> That seems totally contrary to our usual programming practices, and a
> bad idea for that reason.

It is contrary to our usual programming practices, I agree.  I borrowed the
idea from untenured code (da3751c8, 2015-11-11) in load_relcache_init_file():
    if (nailed_rels != NUM_CRITICAL_SHARED_RELS ||        nailed_indexes != NUM_CRITICAL_SHARED_INDEXES)    {
elog(WARNING,"found %d nailed shared rels and %d nailed shared indexes in init file, but expected %d and %d
respectively",            nailed_rels, nailed_indexes,             NUM_CRITICAL_SHARED_RELS,
NUM_CRITICAL_SHARED_INDEXES);       /* Make sure we get developers' attention about this */        Assert(false);
 

I liked this pattern.  It's a good fit for cases that we design to be
impossible yet for which we have a workaround if they do happen.  That being
said, if you feel it's bad, I would be fine using elog(FATAL).  I envision
this as a master-only change in any case.  No PGXN module references
PORTAL_ACTIVE or MarkPortalActive(), so it's unlikely that extension code will
notice the change whether in Assert() form or in elog() form.  What is best?