pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Justin Pryzby <pryzby@telsasoft.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Subject: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables
Date: Wed, 8 Apr 2020 10:24:12 -0500
Message-ID: <20200408152412.GZ2228@telsasoft.com> (raw)

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
pre-existing, matching partition as inherited rather than creating a new one.

DROP TABLE t, t1;
CREATE TABLE t(i int)PARTITION BY RANGE(i);
CREATE TABLE t1 PARTITION OF t FOR VALUES FROM(1)TO(2);
CREATE OR REPLACE FUNCTION trigf() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN END $$;
CREATE TRIGGER trig AFTER INSERT ON t FOR EACH ROW EXECUTE FUNCTION trigf();
SELECT tgrelid::regclass, * FROM pg_trigger WHERE tgrelid='t1'::regclass;
ALTER TABLE t DETACH PARTITION t1;
ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2);
ERROR:  trigger "trig" for relation "t1" already exists

DROP TRIGGER trig ON t1;
ERROR:  cannot drop trigger trig on table t1 because trigger trig on table t requires it
HINT:  You can drop trigger trig on table t instead.

I remember these, but they don't seem to be relevant to this issue, which seems
to be independant.

1fa846f1c9 Fix cloning of row triggers to sub-partitions
b9b408c487 Record parents of triggers

The commit for partitioned indexes talks about using an pre-existing index on
the child as a "convenience gadget", puts indexes into pg_inherit, and
introduces "ALTER INDEX..ATTACH PARTITION" and "CREATE INDEX..ON ONLY".

It's probably rare for a duplicate index to be useful (unless rebuilding to be
more optimal, which is probably not reasonably interspersed with altering
inheritence).  But I don't know if that's equally true for triggers.  So I'm
not sure what the intended behavior is, so I've stopped after implementing
a partial fix.

Attachments:

  [text/x-diff] 0001-WIP-fix-detaching-tables-with-inherited-triggers.patch (1.8K, ../20200408152412.GZ2228@telsasoft.com/2-0001-WIP-fix-detaching-tables-with-inherited-triggers.patch)
  download | inline diff:
From 2c31cac22178d904ee108b77f316886d1e2f6288 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Fri, 3 Apr 2020 22:43:26 -0500
Subject: [PATCH] WIP: fix detaching tables with inherited triggers

---
 src/backend/commands/tablecmds.c | 33 ++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 037d457c3d..10a60e158f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16797,6 +16797,39 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	}
 	table_close(classRel, RowExclusiveLock);
 
+	/* detach triggers too */
+	{
+		/* XXX: relcache.c */
+		ScanKeyData skey;
+		SysScanDesc	scan;
+		HeapTuple	trigtup;
+		Relation	tgrel = table_open(TriggerRelationId, RowExclusiveLock);
+
+		ScanKeyInit(&skey, Anum_pg_trigger_tgrelid, BTEqualStrategyNumber,
+				F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(partRel)));
+
+		scan = systable_beginscan(tgrel, TriggerRelidNameIndexId,
+				true, NULL, 1, &skey);
+
+		while (HeapTupleIsValid(trigtup = systable_getnext(scan)))
+		{
+			Form_pg_trigger pg_trigger;
+			trigtup = heap_copytuple(trigtup);  /* need a modifiable copy */
+			pg_trigger = (Form_pg_trigger) GETSTRUCT(trigtup);
+			/* Set the trigger's parent to Invalid */
+			if (!OidIsValid(pg_trigger->tgparentid))
+				continue;
+			if (!pg_trigger->tgisinternal)
+				continue;
+			pg_trigger->tgparentid = InvalidOid;
+			pg_trigger->tgisinternal = false;
+			CatalogTupleUpdate(tgrel, &trigtup->t_self, trigtup);
+			heap_freetuple(trigtup);
+		}
+		systable_endscan(scan);
+		table_close(tgrel, RowExclusiveLock);
+	}
+
 	/*
 	 * Detach any foreign keys that are inherited.  This includes creating
 	 * additional action triggers.
-- 
2.17.0

view thread (22+ messages)  latest in thread

Message-ID: <20200408152412.GZ2228@telsasoft.com>
Permalink:  ../20200408152412.GZ2228@telsasoft.com/
Also on:    postgresql.org/message-id/20200408152412.GZ2228@telsasoft.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: pryzby@telsasoft.com, pgsql-hackers@lists.postgresql.org, alvherre@alvh.no-ip.org
  Subject: Re: DETACH PARTITION and FOR EACH ROW triggers on partitioned tables
  In-Reply-To: <20200408152412.GZ2228@telsasoft.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox