Hi.
On 2018/06/19 1:59, Alvaro Herrera wrote:
> What does worry me a little bit now, reading this discussion, is whether
> we've made the triggers in partitions visible enough. We'll have this
> problem once we implement BEFORE ROW triggers as proposed, and I think
> we already have this problem now. Let's suppose a user creates a
> duplicated after trigger:
>
> create table parent (a int) partition by range (a);
> create table child partition of parent for values from (0) to (100);
> create function noise() returns trigger language plpgsql as $$ begin raise notice 'nyaa'; return null; end; $$;
> create trigger trig_p after insert on parent for each row execute procedure noise();
> create trigger trig_c after insert on child for each row execute procedure noise();
>
> Now let's try it:
>
> alvherre=# insert into child values (1);
> NOTICE: nyaa
> NOTICE: nyaa
> INSERT 0 1
>
> OK, so where does that one come from?
>
> alvherre=# \d child
> Tabla «public.child»
> Columna │ Tipo │ Collation │ Nullable │ Default
> ─────────┼─────────┼───────────┼──────────┼─────────
> a │ integer │ │ │
> Partition of: parent FOR VALUES FROM (0) TO (100)
> Triggers:
> trig_c AFTER INSERT ON child FOR EACH ROW EXECUTE PROCEDURE noise()
>
> Hmm, there's only one trigger here, why does it appear twice? To find
> out, you have to know where to look:
>
> alvherre=# select tgname, tgrelid::regclass, tgisinternal from pg_trigger;
> tgname │ tgrelid │ tgisinternal
> ────────┼─────────┼──────────────
> trig_p │ parent │ f
> trig_p │ child │ t
> trig_c │ child │ f
> (3 filas)
>
> So there is a trigger in table child, but it's hidden because
> tgisinternal. Of course, you can see it if you look at the parent's
> definition:
>
> alvherre=# \d parent
> Tabla «public.parent»
> Columna │ Tipo │ Collation │ Nullable │ Default
> ─────────┼─────────┼───────────┼──────────┼─────────
> a │ integer │ │ │
> Partition key: RANGE (a)
> Triggers:
> trig_p AFTER INSERT ON parent FOR EACH ROW EXECUTE PROCEDURE noise()
> Number of partitions: 1 (Use \d+ to list them.)
>
> I think it'd be useful to have a list of triggers that have been
> inherited from ancestors, or maybe simply a list of internal triggers
In CreateTrigger(), 86f575948c7 did this.
- values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal);
+ values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal ||
in_partition);
I'm not sure why it had to be done, but undoing this change doesn't seem
to break any regression tests (neither those added by 86f575948c7 nor of
the partitioned table foreign key commit). Did we really intend to change
the meaning of tginternal like this here?
Thanks,
Amit