On Mon, Jun 18, 2018 at 10:29 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > On 2018-Jun-18, Ashutosh Bapat wrote: > >> That's a wrong comparison. Inheritance based partitioning works even >> after declarative partitioning feature is added. So, users can >> continue using inheritance based partitioning if they don't want to >> move to declarative partitioning. That's not true here. A user creates >> a BEFORE ROW trigger on each partition that mimics partitioned table >> level BEFORE ROW trigger. The proposed BEFORE ROW trigger on >> partitioned table will cascade the trigger down to each partition. If >> that fails to recognize that there is already an equivalent trigger, a >> partition table will get two triggers doing the same thing silently >> without any warning or notice. That's fine if the trigger changes the >> salaries to $50K but not OK if each of those increases salary by 10%. >> The database has limited ability to recognize what a trigger is doing. > > I agree with Robert that nobody in their right minds would be caught by > this problem by adding new triggers without thinking about it and > without testing them. If you add a partitioned-table-level trigger to > raise salaries by 10% but there was already one in the partition level > that does the same thing, you'll readily notice that salaries have been > increased by 21% instead. > > So like Robert I'm inclined to not change the wording in the > documentation. > > 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
That's a very good description of the problem people will face. Thanks for elaborating it this way. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
