Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Tue, Apr 21, 2020 at 07:03:30PM -0400, Alvaro Herrera wrote: > On 2020-Apr-20, Justin Pryzby wrote: > > > On Mon, Apr 20, 2020 at 06:35:44PM +0900, Amit Langote wrote: > > > > Also, how about, for consistency, making the parent table labeling of > > > the trigger look similar to that for the

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-20, Justin Pryzby wrote: > On Mon, Apr 20, 2020 at 06:35:44PM +0900, Amit Langote wrote: > > Also, how about, for consistency, making the parent table labeling of > > the trigger look similar to that for the foreign constraint, so > > Triggers: > > TABLE "f1" TRIGGER "trig"

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Tue, Apr 21, 2020 at 12:20:38PM -0400, Alvaro Herrera wrote: > diff --git a/doc/src/sgml/ref/alter_table.sgml > b/doc/src/sgml/ref/alter_table.sgml > index 7595e609b5..233905552c 100644 > --- a/doc/src/sgml/ref/alter_table.sgml > +++ b/doc/src/sgml/ref/alter_table.sgml > @@ -941,13 +943,14 @@

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

I think I also owe the attached doc updates. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 7595e609b5..233905552c 100644

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-20, Alvaro Herrera wrote: > + while (HeapTupleIsValid(trigtup = systable_getnext(scan))) > + { > + Form_pg_trigger pg_trigger = (Form_pg_trigger) > GETSTRUCT(trigtup); > + ObjectAddress trig; > + > + /* Ignore triggers that weren't cloned

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

> + deleteDependencyRecordsFor(TriggerRelationId, > + pg_trigger->oid, > + false); > + deleteDependencyRecordsFor(RelationRelationId, > +

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Mon, Apr 20, 2020 at 06:35:44PM +0900, Amit Langote wrote: > On Mon, Apr 20, 2020 at 5:49 AM Justin Pryzby wrote: > > On Sun, Apr 19, 2020 at 03:13:29PM -0400, Alvaro Herrera wrote: > > > What happens if you detach the parent? I mean, should the trigger > > > removal recurse to children? > >

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Mon, Apr 20, 2020 at 5:49 AM Justin Pryzby wrote: > On Sun, Apr 19, 2020 at 03:13:29PM -0400, Alvaro Herrera wrote: > > On 2020-Apr-18, Justin Pryzby wrote: > > > I haven't heard a compelling argument for or against either way. > > > > > > Maybe the worst behavior might be if, during ATTACH,

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Sun, Apr 19, 2020 at 03:13:29PM -0400, Alvaro Herrera wrote: > On 2020-Apr-18, Justin Pryzby wrote: > > I haven't heard a compelling argument for or against either way. > > > > Maybe the worst behavior might be if, during ATTACH, we searched for a > > matching > > trigger, and "merged" it

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-19, Justin Pryzby wrote: > It's probably rare that we'd be inserting into a table old enough to be > detached, and normally that would be ok, but if a trigger were missing, it > would misbehave. In our use-case, we're creating trigger on the parent as a > convenient way to maintain

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Wed, Apr 08, 2020 at 11:44:33AM -0500, Justin Pryzby wrote: > On Wed, Apr 08, 2020 at 12:02:39PM -0400, Alvaro Herrera wrote: > > On 2020-Apr-08, Justin Pryzby wrote: > > > > > This seems to be a bug in master, v12, and (probably) v11, where "FOR > > > EACH FOR" > > > was first allowed on

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-18, Justin Pryzby wrote: > I haven't heard a compelling argument for or against either way. > > Maybe the worst behavior might be if, during ATTACH, we searched for a > matching > trigger, and "merged" it (marked it inherited) if it matched. That could be > bad if someone *wanted*

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

v3 fixes a brown-paper-bag logic error. -- Justin >From b5de1fc71f805bb8c7ec7e77bfce9a604ccd4bc8 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 3 Apr 2020 22:43:26 -0500 Subject: [PATCH v3] fix detaching tables with inherited row triggers The old behavior is buggy, and the intended

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Thu, Apr 09, 2020 at 09:46:38AM -0400, Tom Lane wrote: > Amit Langote writes: > > On Thu, Apr 9, 2020 at 3:09 AM Tom Lane wrote: > >> My point is that so long as you only allow the case of exactly one parent, > >> you can just delete the child trigger, because it must belong to that > >>

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

Amit Langote writes: > On Thu, Apr 9, 2020 at 3:09 AM Tom Lane wrote: >> My point is that so long as you only allow the case of exactly one parent, >> you can just delete the child trigger, because it must belong to that >> parent. As soon as there's any flexibility, you are going to end up >>

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Thu, Apr 9, 2020 at 3:09 AM Tom Lane wrote: > Alvaro Herrera writes: > > On 2020-Apr-08, Tom Lane wrote: > >> I think that #1 would soon lead to needing all the same infrastructure > >> as we have for inherited columns and constraints, ie triggers would need > >> equivalents of attislocal and

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

Alvaro Herrera writes: > On 2020-Apr-08, Tom Lane wrote: >> I think that #1 would soon lead to needing all the same infrastructure >> as we have for inherited columns and constraints, ie triggers would need >> equivalents of attislocal and attinhcount. I don't really want to go >> there, so I'd

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-08, Tom Lane wrote: > Alvaro Herrera writes: > > Hmm. Let's agree to what behavior we want, and then we implement that. > > It seems to me there are two choices: > > > 1. on detach, keep the trigger but make it independent of the trigger on > > parent. (This requires that the

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

Alvaro Herrera writes: > Hmm. Let's agree to what behavior we want, and then we implement that. > It seems to me there are two choices: > 1. on detach, keep the trigger but make it independent of the trigger on > parent. (This requires that the trigger is made dependent on the > trigger on

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On Wed, Apr 08, 2020 at 12:02:39PM -0400, Alvaro Herrera wrote: > On 2020-Apr-08, Justin Pryzby wrote: > > > This seems to be a bug in master, v12, and (probably) v11, where "FOR EACH > > FOR" > > was first allowed on partition tables (86f575948). > > > > I thought this would work like

Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

On 2020-Apr-08, Justin Pryzby wrote: > This seems to be a bug in master, v12, and (probably) v11, where "FOR EACH > FOR" > was first allowed on partition tables (86f575948). > > I thought this would work like partitioned indexes (8b08f7d48), where > detaching > a partition makes its index

DETACH PARTITION and FOR EACH ROW triggers on partitioned tables

This seems to be a bug in master, v12, and (probably) v11, where "FOR EACH FOR" was first allowed on partition tables (86f575948). I thought this would work like partitioned indexes (8b08f7d48), where detaching a partition makes its index non-inherited, and attaching a partition marks a

Re: FOR EACH ROW triggers on partitioned tables

On 2018/04/30 18:38, Ashutosh Bapat wrote: > On Fri, Mar 23, 2018 at 7:19 PM, Alvaro Herrera > wrote: >> Pushed. Thanks for all the review. >> > > https://www.postgresql.org/docs/devel/static/ddl-partitioning.html > > section 5.10.2.3 still says > "Row triggers, if

Re: FOR EACH ROW triggers on partitioned tables

On Fri, Mar 23, 2018 at 7:19 PM, Alvaro Herrera wrote: > Pushed. Thanks for all the review. > https://www.postgresql.org/docs/devel/static/ddl-partitioning.html section 5.10.2.3 still says "Row triggers, if necessary, must be defined on individual partitions, not the

Re: FOR EACH ROW triggers on partitioned tables

Pushed. Thanks for all the review. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: FOR EACH ROW triggers on partitioned tables

Peter Eisentraut wrote: > On 3/21/18 19:18, Alvaro Herrera wrote: > > Here's v8, which addresses all your comments except the doc updates. I > > added a few more tests, too. Thanks for the review! I intend to commit > > this shortly, probably not before Friday to give some more time for > >

Re: FOR EACH ROW triggers on partitioned tables

02c7d543a471c84aee7b75a9f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 22 Mar 2018 12:07:54 -0400 Subject: [PATCH] fixup! Allow FOR EACH ROW triggers on partitioned tables --- src/include/catalog/pg_constraint.h| 2 +- src/include/commands/trigger.h

Re: FOR EACH ROW triggers on partitioned tables

t, 24x7 Support, Remote DBA, Training & Services >From 5d55e3f752fc90a7a64e426c491493ce548d016e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 21 Nov 2017 15:53:11 -0300 Subject: [PATCH v8] Allow FOR EACH ROW triggers on partitioned tables --- src/backend/

Re: FOR EACH ROW triggers on partitioned tables

On 3/9/18 15:41, Alvaro Herrera wrote: >> One thing I'd like to add before claiming this committable (backend- >> side) is enabling constraint triggers. AFAICT that requires a bit of >> additional logic, but it shouldn't be too terrible. This would allow >> for deferrable unique constraints, for

Re: FOR EACH ROW triggers on partitioned tables

On 3/9/18 16:05, Alvaro Herrera wrote: > ... and this little addendum makes pg_dump work correctly. The header file says "recursing", but the .c file calls the argument "in_partition". -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA,

Re: FOR EACH ROW triggers on partitioned tables

On Fri, Mar 9, 2018 at 7:06 AM, Alvaro Herrera wrote: > Thomas Munro wrote: >> +create trigger failed after update on parted_trig >> + referencing old table as old_table >> + for each statement execute procedure trigger_nothing(); >> >> It doesn't fail as you apparently

Re: FOR EACH ROW triggers on partitioned tables

Peter Eisentraut wrote: > On 3/7/18 20:57, Alvaro Herrera wrote: > > So, unless someone has a brilliant idea on how to construct a column > > mapping from partitioned table to partition, I'm going back to the > > design I was proposing earlier, ie., creating individual pg_trigger rows > > for each

Re: FOR EACH ROW triggers on partitioned tables

... and this little addendum makes pg_dump work correctly. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 867bbe8f1e..ca0a66753e

Re: FOR EACH ROW triggers on partitioned tables

entIdx, parentTbl); } -- 2.11.0 >From 975902aa033877165d0aaec556edc09bb02808c7 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 21 Nov 2017 15:53:11 -0300 Subject: [PATCH v7 3/3] Allow FOR EACH ROW triggers on partitioned tables --- src

Re: FOR EACH ROW triggers on partitioned tables

Peter Eisentraut wrote: > On 3/7/18 20:57, Alvaro Herrera wrote: > > So, unless someone has a brilliant idea on how to construct a column > > mapping from partitioned table to partition, I'm going back to the > > design I was proposing earlier, ie., creating individual pg_trigger rows > > for each

Re: FOR EACH ROW triggers on partitioned tables

On 3/7/18 20:57, Alvaro Herrera wrote: > So, unless someone has a brilliant idea on how to construct a column > mapping from partitioned table to partition, I'm going back to the > design I was proposing earlier, ie., creating individual pg_trigger rows > for each partition that are essentially

Re: FOR EACH ROW triggers on partitioned tables

Index(parentIdx, parentTbl); } -- 2.11.0 >From 40301760e580901379953f400c8c992917234556 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 21 Nov 2017 15:53:11 -0300 Subject: [PATCH v6 3/3] Allow FOR EACH ROW triggers on partitioned table

Re: FOR EACH ROW triggers on partitioned tables

Alvaro Herrera wrote: > I reserve the right to revise this further, as I'm going to spend a > couple of hours looking at it this afternoon, particularly to see how > concurrent DDL behaves, but I don't see anything obviously wrong with > it. I do now. TLDR; I'm afraid this cute idea crashed and

Re: FOR EACH ROW triggers on partitioned tables

On Thu, Mar 8, 2018 at 6:17 AM, Alvaro Herrera wrote: > Here's another version of this patch. It is virtually identical to the > previous one, except for a small doc update and whitespace changes. What is this test for? +create trigger failed after update on

Re: FOR EACH ROW triggers on partitioned tables

@ -133,6 +144,9 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); * relation, as well as ACL_EXECUTE on the trigger function. For internal * triggers the caller must apply any required permission checks. * + * When called on partitioned tables, FOR EACH ROW trigger

Re: FOR EACH ROW triggers on partitioned tables

On Thu, Feb 22, 2018 at 6:52 PM, Alvaro Herrera wrote: > Alvaro Herrera wrote: >> Another option is to rethink this feature from the ground up: instead of >> cloning catalog rows for each children, maybe we should have the trigger >> lookup code, when running DML on the

Re: FOR EACH ROW triggers on partitioned tables

Amit Langote wrote: > On Fri, Feb 23, 2018 at 11:32 AM, Alvaro Herrera > wrote: > > Uh, wow, how have I missed that all this time! Yes, it probably does. > > I'll rework this tomorrow ... and the already committed index patch too, > > I think. > > BTW, not sure if

Re: FOR EACH ROW triggers on partitioned tables

On Fri, Feb 23, 2018 at 11:32 AM, Alvaro Herrera wrote: > Amit Langote wrote: >> On 2018/02/23 8:52, Alvaro Herrera wrote: >> > We could mitigate the performance loss to some extent by adding more to >> > RelationData. For example, a "is_partition" boolean would help:

Re: FOR EACH ROW triggers on partitioned tables

On 2018/02/23 8:52, Alvaro Herrera wrote: > We could mitigate the performance loss to some extent by adding more to > RelationData. For example, a "is_partition" boolean would help: skip > searching pg_inherits for a relation that is not a partition. Unless I'm missing something, doesn't

Re: FOR EACH ROW triggers on partitioned tables

sc, HeapTuple tgtup, +Trigger **triggers, int *numtrigs, int *maxtrigs); +static int qsort_trigger_cmp(const void *a, const void *b); /* @@ -133,6 +144,9 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); * relation, as well as ACL_EXECUTE

Re: FOR EACH ROW triggers on partitioned tables

On 2/15/18 16:55, Alvaro Herrera wrote: > Amit Langote wrote: >> Do you mean to fire these triggers only if the parent table (not a child >> table/partition) is addressed in the DML, right? If the table directly >> addressed in the DML is a partition whose parent has a row-level trigger, >> then

Re: FOR EACH ROW triggers on partitioned tables

On 2018/02/16 6:55, Alvaro Herrera wrote: > Amit Langote wrote: >> On 2018/02/15 6:26, Alvaro Herrera wrote: >>> Another option is to rethink this feature from the ground up: instead of >>> cloning catalog rows for each children, maybe we should have the trigger >>> lookup code, when running DML

Re: FOR EACH ROW triggers on partitioned tables

Amit Langote wrote: > On 2018/02/15 6:26, Alvaro Herrera wrote: > > Another option is to rethink this feature from the ground up: instead of > > cloning catalog rows for each children, maybe we should have the trigger > > lookup code, when running DML on the child relation (the partition), > >

Re: FOR EACH ROW triggers on partitioned tables

On 2018/02/15 6:26, Alvaro Herrera wrote: > Another option is to rethink this feature from the ground up: instead of > cloning catalog rows for each children, maybe we should have the trigger > lookup code, when running DML on the child relation (the partition), > obtain trigger entries not only

Re: FOR EACH ROW triggers on partitioned tables

NEW; end if; -- 2.11.0 >From 2ba9e8eaa53284eed1eefb137501ed1a22f4320b Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 21 Nov 2017 15:53:11 -0300 Subject: [PATCH v3 2/3] Allow FOR EACH ROW triggers on partitioned tables --- src/bac

Re: FOR EACH ROW triggers on partitioned tables

Moved to next commit fest. There is some work to be done, but there appears to be a straight path to success. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [Sender Address Forgery]Re: FOR EACH ROW triggers on partitioned tables

On 2018/01/31 9:44, Peter Eisentraut wrote: > On 1/30/18 04:49, Amit Langote wrote: >>> If you are writing a generic >>> trigger function, maybe to dump out all columns, you want to know the >>> physical table and its actual columns. It's easy[citation needed] to >>> get the partition root for a

Re: FOR EACH ROW triggers on partitioned tables

On 1/30/18 04:49, Amit Langote wrote: >> If you are writing a generic >> trigger function, maybe to dump out all columns, you want to know the >> physical table and its actual columns. It's easy[citation needed] to >> get the partition root for a given table, if the trigger code needs >> that.

Re: FOR EACH ROW triggers on partitioned tables

On 2018/01/30 5:30, Peter Eisentraut wrote: > On 1/23/18 17:10, Alvaro Herrera wrote: >> The main question is this: when running the trigger function, it is >> going to look as it is running in the context of the partition, not in >> the context of the parent partitioned table (TG_RELNAME etc).

Re: FOR EACH ROW triggers on partitioned tables

On 1/23/18 17:10, Alvaro Herrera wrote: > The main question is this: when running the trigger function, it is > going to look as it is running in the context of the partition, not in > the context of the parent partitioned table (TG_RELNAME etc). That > seems mildly ugly: some users may be

Re: FOR EACH ROW triggers on partitioned tables

On Tue, Jan 23, 2018 at 5:10 PM, Alvaro Herrera wrote: > The main question is this: when running the trigger function, it is > going to look as it is running in the context of the partition, not in > the context of the parent partitioned table (TG_RELNAME etc). That >

Re: FOR EACH ROW triggers on partitioned tables

On 3 January 2018 at 03:12, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 12/29/17 17:53, Alvaro Herrera wrote: >> This patch enables FOR EACH ROW triggers on partitioned tables. >> >> As presented, this patch is sufficient to discuss the semantics t

Re: FOR EACH ROW triggers on partitioned tables

On 12/29/17 17:53, Alvaro Herrera wrote: > This patch enables FOR EACH ROW triggers on partitioned tables. > > As presented, this patch is sufficient to discuss the semantics that we > want for triggers on partitioned tables, which is the most pressing > question here ISTM. Th

FOR EACH ROW triggers on partitioned tables

This patch enables FOR EACH ROW triggers on partitioned tables. As presented, this patch is sufficient to discuss the semantics that we want for triggers on partitioned tables, which is the most pressing question here ISTM. However, this is incomplete: it doesn't create triggers when you do