Re: BUG #17607: Server process crashes when PLpgSQL function raises error in subtransaction

I wrote:
> It seems to me there are probably other hazards here, because the tupdesc
> could possibly also go away before the slot does.  I think what we ought
> to do is copy the tupdesc, so that we have a non-refcounted descriptor
> that we know has exactly the right lifespan.  As attached.

Actually, after looking around some more, I realize that there is a
second mistake in 25936fd46: it ignored the comment on
AfterTriggerFreeQuery that

 *    Note: it's important for this to be safe if interrupted by an error
 *    and then called again for the same query level.

This is the reason why we end up in a recursive error and PANIC:
we keep trying to free the tupdesc again after the previous error.
If I fix that but omit the CreateTupleDescCopy step, then the
reproducer behaves much more sanely:

psql:bug17607.sql:29: WARNING:  AbortSubTransaction while in ABORT state
psql:bug17607.sql:29: ERROR:  BOOM !
CONTEXT:  PL/pgSQL function on_tbl_parent_id_change_fn() line 3 at RAISE
ERROR:  tupdesc reference 0x7fdef236a1b8 is not owned by resource owner SubTransaction
ROLLBACK
ROLLBACK

So what we actually need here is more like the attached.

            regards, tom lane

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 59b38d00ed..1b1193997b 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4340,11 +4340,13 @@ GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
         MemoryContext    oldcxt;

         /*
-         * We only need this slot only until AfterTriggerEndQuery, but making
-         * it last till end-of-subxact is good enough.  It'll be freed by
-         * AfterTriggerFreeQuery().
+         * We need this slot only until AfterTriggerEndQuery, but making it
+         * last till end-of-subxact is good enough.  It'll be freed by
+         * AfterTriggerFreeQuery().  However, the passed-in tupdesc might have
+         * a different lifespan, so we'd better make a copy of that.
          */
         oldcxt = MemoryContextSwitchTo(CurTransactionContext);
+        tupdesc = CreateTupleDescCopy(tupdesc);
         table->storeslot = MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual);
         MemoryContextSwitchTo(oldcxt);
     }
@@ -4640,7 +4642,12 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
         if (ts)
             tuplestore_end(ts);
         if (table->storeslot)
-            ExecDropSingleTupleTableSlot(table->storeslot);
+        {
+            TupleTableSlot *slot = table->storeslot;
+
+            table->storeslot = NULL;
+            ExecDropSingleTupleTableSlot(slot);
+        }
     }

     /*