Re: delta relations in AFTER triggers

Thomas Munro <thomas.munro@enterprisedb.com>

From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Kevin Grittner <kgrittn@gmail.com>
Cc: Haribabu Kommi <kommi.haribabu@gmail.com>, Jim Nasby <Jim.Nasby@bluetreble.com>, Craig Ringer <craig@2ndquadrant.com>, Michael Paquier <michael.paquier@gmail.com>, Robert Haas <robertmhaas@gmail.com>, David Fetter <david@fetter.org>, Kevin Grittner <kgrittn@ymail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Amit Khandekar <amit.khandekar@enterprisedb.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-03-26T11:39:49Z
Lists: pgsql-hackers

Attachments

On Fri, Mar 24, 2017 at 1:14 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Tue, Mar 14, 2017 at 7:51 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
>> On Sun, Mar 12, 2017 at 4:08 PM, Thomas Munro
>> <thomas.munro@enterprisedb.com> wrote:
>>> I found a new way to break it: run the trigger function so
>>> that the plan is cached by plpgsql, then ALTER TABLE incompatibly,
>>> then run the trigger function again.  See attached.
>>
>> [...]
>>
>> I expected that existing mechanisms would have forced re-planning of
>> a trigger function if the table the function was attached to was
>> altered.  Either that was a bit "optimistic", or the old TupleDesc
>> is used for the new plan.  Will track down which it is, and fix it.
>
> When PlanCacheRelCallback runs, I don't think it understands that
> these named tuplestore RangeTblEntry objects are dependent on the
> subject table.  Could that be fixed like this?
>
> @@ -2571,6 +2582,9 @@ extract_query_dependencies_walker(Node *node,
> PlannerInfo *context)
>                         if (rte->rtekind == RTE_RELATION)
>                                 context->glob->relationOids =
>
> lappend_oid(context->glob->relationOids, rte->relid);
> +                       else if (rte->rtekind == RTE_NAMEDTUPLESTORE)
> +                               context->glob->relationOids =
> +
> lappend_oid(context->glob->relationOids, [subject table's OID]);

I'm not sure if this is the right approach and it may have style
issues, but it does fix the crashing in the ALTER TABLE case I
reported: see attached patch which applies on top of your v12.

BTW I had to make the following change to your v12 because of commit b8d7f053:

     /*
      * initialize child expressions
      */
-    scanstate->ss.ps.targetlist = (List *)
-        ExecInitExpr((Expr *) node->scan.plan.targetlist,
-                     (PlanState *) scanstate);
-    scanstate->ss.ps.qual = (List *)
-        ExecInitExpr((Expr *) node->scan.plan.qual,
-                     (PlanState *) scanstate);
+    scanstate->ss.ps.qual =
+        ExecInitQual(node->scan.plan.qual, (PlanState *) scanstate);

-- 
Thomas Munro
http://www.enterprisedb.com

Commits

  1. Don't permit transition tables with TRUNCATE triggers.

  2. Pass EXEC_FLAG_REWIND when initializing a tuplestore scan.

  3. Faster expression evaluation and targetlist projection.

  4. Remove obsoleted code relating to targetlist SRF evaluation.

  5. Implement syntax for transition tables in AFTER triggers.