On 2018/06/28 7:58, Alvaro Herrera wrote:
> On 2018-Jun-18, Robert Treat wrote:
>> 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.
>
> The firing order is alphabetical across the whole set, i.e. parent/child
> triggers are interleaved. See the regression test output at the bottom
> of this email.
>
> I looked into adding a "Partition triggers" section because it seemed a
> nice idea, but looking at the code I realized it's a bit tough because
> we already split triggers in "categories": normal triggers, disabled
> triggers, ALWAYS triggers and REPLICA triggers (src/bin/psql/describe.c
> line 2795). Since the trigger being in a partition is orthogonal to
> that classification, we would end up with eight groups. Not sure that's
> great.
>
> The attached patch (catversion bump not included -- beware of server
> crash if you run it without initdb'ing) keeps the categories the same.
Thanks for creating the patch.
> So with my previous example setup, you can see the duplicate triggers in
> psql:
>
> alvherre=# \d child
> Table "public.child"
> Column │ Type │ 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()
> trig_p AFTER INSERT ON child FOR EACH ROW EXECUTE PROCEDURE noise()
>
> and as soon as you try to drop the second one, you'll learn the truth
> about it:
>
> alvherre=# drop trigger trig_p on child;
> ERROR: cannot drop trigger trig_p on table child because trigger trig_p on
> table parent requires it
> SUGERENCIA: You can drop trigger trig_p on table parent instead.
> Time: 1,443 ms
>
> I say this is sufficient.
Yeah. We do same with constraints for example:
create table p (a int check (a >= 0), b int) partition by list (a);
create table p1 partition of p (b check (b >= 0)) for values in (1);
\d p1
Table "public.p1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition of: p FOR VALUES IN (1)
Check constraints:
"p1_b_check" CHECK (b >= 0)
"p_a_check" CHECK (a >= 0)
alter table p1 drop constraint p_a_check;
ERROR: cannot drop inherited constraint "p_a_check" of relation "p1"
More specifically, inherited triggers are not listed separately.
By the way, picking up on the word "inherited" in the error message shown
above, I wonder if you decided against using similar terminology
intentionally. I guess the thinking there is that the terminology being
used extensively with columns and constraints ("inherited column/check
constraint cannot be dropped", etc.) is just a legacy of partitioning
sharing implementation with inheritance.
Thanks,
Amit
