Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@postgresql.org, Amit Langote <amitlangote09@gmail.com>
Date: 2020-10-20T19:44:12Z
Lists: pgsql-hackers
On Tue, Oct 20, 2020 at 04:04:20PM -0300, Alvaro Herrera wrote:
> On 2020-Sep-30, Justin Pryzby wrote:
>
> > CREATE TABLE t(i int) PARTITION BY RANGE(i);
> > CREATE TABLE t1 PARTITION OF t FOR VALUES FROM (1) TO (10);
> > CREATE OR REPLACE FUNCTION tgf() RETURNS trigger LANGUAGE plpgsql AS $$ begin raise exception 'except'; end $$;
> > CREATE TRIGGER tg AFTER INSERT ON t FOR EACH ROW EXECUTE FUNCTION tgf();
> > ALTER TABLE t1 DISABLE TRIGGER tg;
> > INSERT INTO t VALUES(1); -- inserts when trigger is disabled: good
> > ALTER TABLE t DISABLE TRIGGER tg;
> > CREATE TABLE t2 PARTITION OF t FOR VALUES FROM (10) TO (20);
> >
> > postgres=# SELECT tgrelid::regclass, tgenabled FROM pg_trigger WHERE tgrelid::regclass::text IN ('t1','t2');
> > tgrelid | tgenabled
> > ---------+-----------
> > t1 | D
> > t2 | O
> > (2 rows)
> >
> > I consider this a bug,but CreateTrigStmt doesn't have any "enabled" member
> > (since it's impossible to CREATE TRIGGER .. DISABLED), so I'm not sure where
> > the fix should be.
>
> Hmm, next question: should we backpatch a fix for this? (This applies
> all the way back to 11.) If we do, then we would change behavior of
> partition creation. It's hard to see that the current behavior is
> desirable ... and I think anybody who would have come across this, would
> wish it behaved the other way. But still -- it would definitely be a
> behavior change.
+0.8 to backpatch. To v13 if not further.
We don't normally disable triggers, otherwise I would say +1.
For context, I ran into this issue while migrating a customer to a new server
using pg_restore and a custom backup script which loops around pg_dump, and
handles partitioned tables differently depending if they're recent or historic.
Our backup job works well, but is technically a bit of a hack. It doesn't do
the right thing (causes sql errors and pg_restore warnings) for inherited
indexes and, apparently, triggers. Disabling the trigger was my 4th attempt to
handle an error restoring a specific table (mismatched column type between
parent dump and child dumped several days earlier). I eventually (5th
or 6th attempt) dropped the parent trigger, created the child tables using
--section=pre-data, ALTERed a column to match, and then ran post-data and
attached it.
--
Justin
Commits
-
Fix pg_dump for disabled triggers on partitioned tables
- f0e21f2f6167 15.0 landed
- ccfc3cbb341a 11.13 landed
- cc340af33453 13.4 landed
- 5992c94dc7e4 12.8 landed
- 3c5b7c628621 14.0 landed
-
Preserve firing-on state when cloning row triggers to partitions
- fed35bd4a650 11.13 landed
- eef92de11e50 14.0 landed
- df80fa2ee504 15.0 landed
- c31516ae5b43 13.4 landed
- 7584ec1f6098 12.8 landed
-
Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion
- bbb927b4db9b 14.0 landed
- a795f6782fa8 11.10 landed
- 5f6463a20af1 13.1 landed
- 0e6b6f8c7192 12.5 landed
-
psql \d: Display table where trigger is defined, if inherited
- c33869cc3bfc 13.0 cited
-
Fix cloning of row triggers to sub-partitions
- 1fa846f1c9af 13.0 cited
-
Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)
- 33a53130a894 12.0 cited