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

On 2018/05/02 14:17, Ashutosh Bapat wrote:
> On Tue, May 1, 2018 at 5:20 PM, Peter Eisentraut
> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>> On 4/26/18 05:03, Amit Langote wrote:
>>> On 2018/04/26 13:01, David Rowley wrote:
>>>> The attached small patch removes the mention that partitioned tables
>>>> cannot have foreign keys defined on them.
>>>>
>>>> This has been supported since 3de241db
>>>
>>> I noticed also that the item regarding row triggers might be obsolete as
>>> of 86f575948c7, thanks again to Alvaro! So, I updated your patch to take
>>> care of that.
>>
>> committed both
>
> AFAIK we still don't support BEFORE ROW triggers, so removing that
> entry altogether is misleading.

You're right. I think that's what you were also saying on the other
thread, in reply to which I directed you to this thread. I very clearly
missed that BEFORE ROW triggers are still disallowed.

create trigger br_insert_trig before insert on p for each row execute
procedure br_insert_trigfunc();
ERROR: "p" is a partitioned table
DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.

Here is a patch that adds back a line to state this particular limitation.
Sorry about the earlier misleading patch.

Fwiw, I am a bit surprised to see the error, but that's irrelevant now. I
see that 86f575948c7 added the following comment in CreateTrigger() above
the check that causes above error.

/*
* BEFORE triggers FOR EACH ROW are forbidden, because they would
* allow the user to direct the row to another partition, which
* isn't implemented in the executor.
*/

But if that's the only reason, I think it might be too restrictive.
Allowing them would not lead to something wrong happening, afaics. To
illustrate, I temporarily removed the check to allow BR triggers to be
created on the parent and thus being cloned to each partition:

create table p (a int) partition by list (a);
create table p1 partition of p for values in (1);

create or replace function br_insert_trigfunc() returns trigger as
$$ begin new.a := new.a + 1; return new; end $$
language plpgsql;

create trigger br_insert_trig before insert on p for each row execute
procedure br_insert_trigfunc();

insert into p values (1, 'a') returning *;
ERROR: new row for relation "p1" violates partition constraint
DETAIL: Failing row contains (2, a).

Since the same error would occur if the trigger were instead defined
directly on the partition (which *is* allowed), maybe users could live
with this. But one could very well argue that BEFORE ROW triggers on the
parent should run before performing the tuple routing and not be cloned to
individual partitions, in which case the result would not have been the
same. Perhaps that's the motivation behind leaving this to be considered
later.

Thanks,
Amit