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
