On Thu, Mar 23, 2017 at 11:36 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> One more thought: should this be allowed?
>
> postgres=# create table mytab (i int) partition by list (i);
> CREATE TABLE
> postgres=# create table mytab1 partition of mytab for values in (42);
> CREATE TABLE
> postgres=# create function my_trigger_function() returns trigger as $$
> begin end; $$ language plpgsql;
> CREATE FUNCTION
> postgres=# create trigger my_trigger after update on mytab referencing
> old table as my_old for each statement execute procedure
> my_trigger_function();
> CREATE TRIGGER
> Perhaps the moral equivalent should be possible for statement triggers
> with transition tables, and that already works with your patch as far
> as I know. So I think your patch probably just needs to reject them
> on partitioned tables.
> [patch provided]
Yeah, that looks good. Included in next patch version.
On Sun, Mar 26, 2017 at 6:39 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> BTW I had to make the following change to your v12 because of commit b8d7f053:
Yeah, I ran into that, too, and used exactly the same fix.
On Sun, Mar 26, 2017 at 6:39 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Fri, Mar 24, 2017 at 1:14 PM, Thomas Munro
>> 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.
I had been working along similar lines, but had not gotten it
working. Merged your version and mine, taking the best of both.
:-)
Thanks for the reviews and the fixes!
New version attached. It needs some of these problem cases added to
the testing, and a mention in the docs that only C and plpgsql
triggers can use the feature so far. I'll add those tomorrow.
--
Kevin Grittner
