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

2020-04-21 Thread Justin Pryzby
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

2020-04-21 Thread Alvaro Herrera
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

2020-04-21 Thread Justin Pryzby
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

2020-04-21 Thread Alvaro Herrera
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

2020-04-21 Thread Alvaro Herrera
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

2020-04-20 Thread Alvaro Herrera
> + deleteDependencyRecordsFor(TriggerRelationId, > + pg_trigger->oid, > + false); > + deleteDependencyRecordsFor(RelationRelationId, > +

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

2020-04-20 Thread Justin Pryzby
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

2020-04-20 Thread Amit Langote
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

2020-04-19 Thread Justin Pryzby
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

2020-04-19 Thread Alvaro Herrera
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

2020-04-19 Thread Justin Pryzby
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

2020-04-19 Thread Alvaro Herrera
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

2020-04-18 Thread Justin Pryzby
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

2020-04-18 Thread Justin Pryzby
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

2020-04-09 Thread Tom Lane
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

2020-04-09 Thread Amit Langote
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

2020-04-08 Thread Tom Lane
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

2020-04-08 Thread Alvaro Herrera
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

2020-04-08 Thread Tom Lane
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

2020-04-08 Thread Justin Pryzby
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

2020-04-08 Thread Alvaro Herrera
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

2020-04-08 Thread Justin Pryzby
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