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

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.

Thanks,
Amit

On 26 April 2018 at 21:03, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

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.

Thanks. I walked right past that one.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 1 May 2018 at 23:50, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

committed both

Thanks!

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2018/05/01 20:50, Peter Eisentraut 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

Thank you.

Regards,
Amit

On Tue, May 1, 2018 at 5:20 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.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.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 2018/05/02 14:17, Ashutosh Bapat wrote:

On Tue, May 1, 2018 at 5:20 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.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

On Wed, May 2, 2018 at 11:56 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

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.

+1 for that. We should associate before row triggers with the
partitioned table and not with the partitions. Those should be
executed before tuple routing (for that partition level) kicks in. But
then that would be asymetric with AFTER row trigger behaviour. From
this POV, pushing AFTER row triggers to the individual partitions
doesn't look good.

Other question that comes to my mind is what happens when rows are
inserted into a partition directly. Do we execute BEFORE trigger on
the partitioned table?

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 5/2/18 02:26, Amit Langote wrote:

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.

fixed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, May 2, 2018 at 9:17 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

On Wed, May 2, 2018 at 11:56 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

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.

+1 for that. We should associate before row triggers with the
partitioned table and not with the partitions. Those should be
executed before tuple routing (for that partition level) kicks in. But
then that would be asymetric with AFTER row trigger behaviour. From
this POV, pushing AFTER row triggers to the individual partitions
doesn't look good.

The asymmetry doesn't seem horrible to me on its own merits, but it
would lead to some odd behavior: suppose you defined a BEFORE ROW
trigger and an AFTER ROW trigger on the parent, and then inserted one
row into the parent table and a second row directly into a partition.
It seems like the BEFORE ROW trigger would fire only for the parent
insert, but the AFTER ROW trigger would fire in both cases.

What seems like a better idea is to have the BEFORE ROW trigger get
cloned to each partition just as we do with AFTER ROW triggers, but
arrange things so that if it already got fired for the parent table,
it doesn't get fired again after tuple routing. This would be a bit
tricky to get correct in cases where there are multiple levels of
partitioning involved, but it seems doable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2018-May-03, Robert Haas wrote:

The asymmetry doesn't seem horrible to me on its own merits, but it
would lead to some odd behavior: suppose you defined a BEFORE ROW
trigger and an AFTER ROW trigger on the parent, and then inserted one
row into the parent table and a second row directly into a partition.
It seems like the BEFORE ROW trigger would fire only for the parent
insert, but the AFTER ROW trigger would fire in both cases.

What seems like a better idea is to have the BEFORE ROW trigger get
cloned to each partition just as we do with AFTER ROW triggers, but
arrange things so that if it already got fired for the parent table,
it doesn't get fired again after tuple routing. This would be a bit
tricky to get correct in cases where there are multiple levels of
partitioning involved, but it seems doable.

Hmm. I'm adding this to the open items list. I'll study this next
week.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018-May-03, Robert Haas wrote:

On Wed, May 2, 2018 at 9:17 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

+1 for that. We should associate before row triggers with the
partitioned table and not with the partitions. Those should be
executed before tuple routing (for that partition level) kicks in. But
then that would be asymetric with AFTER row trigger behaviour. From
this POV, pushing AFTER row triggers to the individual partitions
doesn't look good.

The asymmetry doesn't seem horrible to me on its own merits, but it
would lead to some odd behavior: suppose you defined a BEFORE ROW
trigger and an AFTER ROW trigger on the parent, and then inserted one
row into the parent table and a second row directly into a partition.
It seems like the BEFORE ROW trigger would fire only for the parent
insert, but the AFTER ROW trigger would fire in both cases.

This kind of inconsistency is what I wanted to avoid. One of the
guiding principles here was that a partitioned table behaves just like a
regular table does; in particular, inserting directly into a partition
is an application-level optimization that must behave exactly like it
would if the insert had gone into its parent table (unless the user
explicitly makes it not so). If we make an insertion into the partition
*not* fire the trigger that would have been fired by inserting into the
partitioned table, that's a bug. I couldn't see a way to have the
BEFORE trigger handle that nicely, so I decided to punt on that feature.

So I stand by my original decision to disallow BEFORE triggers on
partitioned tables altogether -- we can add that as a new feature later,
keeping in mind the above ... namely to implement Robert's idea:

What seems like a better idea is to have the BEFORE ROW trigger get
cloned to each partition just as we do with AFTER ROW triggers, but
arrange things so that if it already got fired for the parent table,
it doesn't get fired again after tuple routing. This would be a bit
tricky to get correct in cases where there are multiple levels of
partitioning involved, but it seems doable.

In the meantime, my inclination is to fix the documentation to point out
that AFTER triggers are allowed but BEFORE triggers are not.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

This kind of inconsistency is what I wanted to avoid. One of the
guiding principles here was that a partitioned table behaves just like a
regular table does; in particular, inserting directly into a partition
is an application-level optimization that must behave exactly like it
would if the insert had gone into its parent table (unless the user
explicitly makes it not so). If we make an insertion into the partition
*not* fire the trigger that would have been fired by inserting into the
partitioned table, that's a bug. I couldn't see a way to have the
BEFORE trigger handle that nicely, so I decided to punt on that feature.

Reasonable, but ...

In the meantime, my inclination is to fix the documentation to point out
that AFTER triggers are allowed but BEFORE triggers are not.

... why doesn't the same problem apply to AFTER triggers that are attached
to the inheritance parent?

regards, tom lane

On 2018-Jun-04, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

This kind of inconsistency is what I wanted to avoid. One of the
guiding principles here was that a partitioned table behaves just like a
regular table does; in particular, inserting directly into a partition
is an application-level optimization that must behave exactly like it
would if the insert had gone into its parent table (unless the user
explicitly makes it not so). If we make an insertion into the partition
*not* fire the trigger that would have been fired by inserting into the
partitioned table, that's a bug. I couldn't see a way to have the
BEFORE trigger handle that nicely, so I decided to punt on that feature.

Reasonable, but ...

In the meantime, my inclination is to fix the documentation to point out
that AFTER triggers are allowed but BEFORE triggers are not.

... why doesn't the same problem apply to AFTER triggers that are attached
to the inheritance parent?

The trigger is not executed as attached to the parent; it's only
executed as attached to the partition itself. So you create it in the
parent, and clone so that all partitions have it; when you run the
command, only the cloned trigger is run, so in effect the trigger runs
exactly once. Because the trigger runs *after* the target partition has
been selected, and the trigger cannot change that outcome (ie. it
cannot move the row to another partition) this works fine.

With a BEFORE trigger, running the trigger might change the target
partition, which has the potential for all kinds of trouble. Also,
there's room to say that the before trigger should run before partition
routing is even invoked (hence the idea of running the triggers in the
partitioned table rather than the partition). But in that case we must
not run the trigger again when executing the insertion in the chosen
partition; and we must do *something* if the user executes the command
targetting the partition rather than the parent table.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On 2018-Jun-04, Tom Lane wrote:

... why doesn't the same problem apply to AFTER triggers that are attached
to the inheritance parent?

With a BEFORE trigger, running the trigger might change the target
partition, which has the potential for all kinds of trouble.

Got it. That seems like not just an implementation restriction, but
a pretty fundamental issue.

Could we solve it by saying that triggers on partitioned tables aren't
allowed to change the partitioning values? (Or at least, not allowed
to change them in a way that changes the target partition.)

regards, tom lane

On 2018-Jun-04, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On 2018-Jun-04, Tom Lane wrote:

... why doesn't the same problem apply to AFTER triggers that are attached
to the inheritance parent?

With a BEFORE trigger, running the trigger might change the target
partition, which has the potential for all kinds of trouble.

Got it. That seems like not just an implementation restriction, but
a pretty fundamental issue.

Could we solve it by saying that triggers on partitioned tables aren't
allowed to change the partitioning values? (Or at least, not allowed
to change them in a way that changes the target partition.)

Yes, that would be one way forward. In fact, IIUC what happens today if
you remove the restriction (as Amit tested and reported upthread[1]/messages/by-id/1824bda1-0c47-abc4-8b97-e37414c52f6c@lab.ntt.co.jp) is
that this already causes an error, because tuple routing is not re-done
after BEFORE triggers are run.

I was thinking it would be good to leave the option open (i.e. forbid
BEFORE triggers altogether) so that in the future we could implement
that case too, and avoid a backwards-incompatible behavior change.
Thinking about it again, this may not be a problem: if you write a
trigger that works (it doesn't change the target partition) then when
forward-porting it to a version that does allow the target partition to
change, your trigger doesn't change behavior. So maybe it's okay to
remove the restriction. I'll test more tomorrow.

[1]: /messages/by-id/1824bda1-0c47-abc4-8b97-e37414c52f6c@lab.ntt.co.jp

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Jun 4, 2018 at 1:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Could we solve it by saying that triggers on partitioned tables aren't
allowed to change the partitioning values? (Or at least, not allowed
to change them in a way that changes the target partition.)

That seems like a somewhat-unfortunate restriction.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jun 4, 2018 at 1:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Could we solve it by saying that triggers on partitioned tables aren't
allowed to change the partitioning values? (Or at least, not allowed
to change them in a way that changes the target partition.)

That seems like a somewhat-unfortunate restriction.

Perhaps, but I'm having a hard time wrapping my mind around what the
semantics ought to be. If a trigger on partition A changes the keys
so that the row shouldn't have gone into A at all, what then? That
trigger should never have fired, eh?

regards, tom lane

On Mon, Jun 4, 2018 at 4:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jun 4, 2018 at 1:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Could we solve it by saying that triggers on partitioned tables aren't
allowed to change the partitioning values? (Or at least, not allowed
to change them in a way that changes the target partition.)

That seems like a somewhat-unfortunate restriction.

Perhaps, but I'm having a hard time wrapping my mind around what the
semantics ought to be. If a trigger on partition A changes the keys
so that the row shouldn't have gone into A at all, what then? That
trigger should never have fired, eh?

Causality is for wimps. :-)

I agree that it's weird if you think about a partition, but it's a lot
less strange if you think about a partitioned table. If we're running
the trigger before tuple routing has been done, then we ought to be
able to change the results of tuple routing.

I think, in general, that we should try to pick semantics that make a
partitioned table behave like an unpartitioned table, provided that
all triggers are defined on the partitioned table itself.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company