Re: Remove mention in docs that foreign keys on partitioned tables are not supported

On Mon, Jun 18, 2018 at 12:59 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)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
>
> Or maybe this is not something to worry about?
>

One of the things I was thinking about while reading this thread is
that the scenario of creating "duplicate" triggers on a table meaning
two triggers doing the same thing is entirely possible now but we
don't really do anything to prevent it, which is Ok. I've never seen
much merit in the argument "people should test" (it assumes a certain
infallibility that just isn't true) but we've also generally been
pretty good about exposing what is going on so people can debug this
type of unexpected behavior.

So +1 for thinking we do need to worry about it. I'm not exactly sure
how we want to expose that info; with \d+ we list various "Partition
X:" sections, perhaps adding one for "Partition triggers:" would be
enough, although I am inclined to think it needs exposure at the \d
level. One other thing to consider is firing order of said triggers...
if all parent level triggers fire before child level triggers then the
above separation is straightforward, but if the execution order is, as
I suspect, mixed, then it becomes more complicated.

Robert Treat
http://xzilla.net