Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Justin Pryzby
On Fri, Jul 16, 2021 at 06:01:12PM -0400, Alvaro Herrera wrote: > On 2021-Jul-16, Justin Pryzby wrote: > > CREATE TABLE p(i int) PARTITION BY RANGE(i); > > CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (1)TO(2); > > CREATE FUNCTION foo() returns trigger LANGUAGE plpgsql AS $$begin end$$; > >

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Alvaro Herrera
On 2021-Jul-16, Justin Pryzby wrote: > CREATE TABLE p(i int) PARTITION BY RANGE(i); > CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (1)TO(2); > CREATE FUNCTION foo() returns trigger LANGUAGE plpgsql AS $$begin end$$; > CREATE TRIGGER x AFTER DELETE ON p1 EXECUTE FUNCTION foo(); > CREATE TRIGGER

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Alvaro Herrera
I noticed that in pg_dump --clean, we were still emitting DROP lines for the triggers in the partitions, which raises errors; so I emptied that too. On 2021-Jul-14, Alvaro Herrera wrote: > Looking over the tests added by your (Justin's) patch, there was a > problem checking for non-existance of

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Justin Pryzby
On Fri, Jul 16, 2021 at 02:15:26PM -0400, Tom Lane wrote: > Justin Pryzby writes: > > I think there's still an issue that comments on child triggers aren't > > preserved, right ? > > Do we care? That seems like messing with a system-internal object. > In general we won't promise to preserve the

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Tom Lane
Justin Pryzby writes: > I think there's still an issue that comments on child triggers aren't > preserved, right ? Do we care? That seems like messing with a system-internal object. In general we won't promise to preserve the results of doing so. regards, tom lane

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-16 Thread Justin Pryzby
Thanks for handling this. I think there's still an issue that comments on child triggers aren't preserved, right ? -- Justin

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-14 Thread Alvaro Herrera
Looking over the tests added by your (Justin's) patch, there was a problem checking for non-existance of the CREATE TRIGGER line: it doesn't work to use "like => {}" (empty), you need to use "unlike => { everything }". So your test was not catching the fact that we were, in fact, emitting the

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-14 Thread Zhihong Yu
On Wed, Jul 14, 2021 at 11:02 AM Alvaro Herrera wrote: > On 2020-Oct-27, Justin Pryzby wrote: > > > I think either way could be ok - if you assume that the trigger was > disabled > > with ONLY, then it'd make sense to restore it with ONLY, but I think > it's at > > least as common to ALTER TABLE

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2021-07-14 Thread Alvaro Herrera
On 2020-Oct-27, Justin Pryzby wrote: > I think either way could be ok - if you assume that the trigger was disabled > with ONLY, then it'd make sense to restore it with ONLY, but I think it's at > least as common to ALTER TABLE [*]. It might look weird to the user if they > used ALTER TABLE ONLY

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-27 Thread Justin Pryzby
On Wed, Oct 21, 2020 at 02:02:37PM -0300, Alvaro Herrera wrote: > On 2020-Oct-21, Justin Pryzby wrote: > > > I came up with this, which probably needs more than a little finesse. > > Hmm, there are two important changes needed on this: 1) it must not emit > CREATE lines for the child triggers;

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-21 Thread Alvaro Herrera
On 2020-Oct-21, Justin Pryzby wrote: > I came up with this, which probably needs more than a little finesse. Hmm, there are two important changes needed on this: 1) it must not emit CREATE lines for the child triggers; only the ALTER TABLE ONLY lines to set disable state on the partition are

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-21 Thread Justin Pryzby
On Tue, Oct 20, 2020 at 09:54:53PM -0300, Alvaro Herrera wrote: > On 2020-Oct-20, Justin Pryzby wrote: > > On Tue, Oct 20, 2020 at 03:56:30PM -0400, Tom Lane wrote: > > > Alvaro Herrera writes: > > > > Hmm, next question: should we backpatch a fix for this? (This applies > > > > all the way back

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Alvaro Herrera
On 2020-Oct-20, Justin Pryzby wrote: > On Tue, Oct 20, 2020 at 03:56:30PM -0400, Tom Lane wrote: > > Alvaro Herrera writes: > > > 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

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Alvaro Herrera
On 2020-Oct-20, Alvaro Herrera wrote: > > diff --git a/src/backend/commands/tablecmds.c > > b/src/backend/commands/tablecmds.c > > index 511f015a86..c8d6f78da2 100644 > > --- a/src/backend/commands/tablecmds.c > > +++ b/src/backend/commands/tablecmds.c > > @@ -4321,6 +4321,7 @@ ATPrepCmd(List

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Justin Pryzby
On Tue, Oct 20, 2020 at 03:56:30PM -0400, Tom Lane wrote: > Alvaro Herrera writes: > > 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

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Alvaro Herrera
On 2020-Oct-16, Alvaro Herrera wrote: > On 2020-Oct-16, Alvaro Herrera wrote: > > > I also just noticed that ALTER TABLE ONLY recurses to children, which it > > should not. > > Apparently I wrote (bogus) bespoke code to handle recursion in > EnableDisableTrigger instead of using

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Tom Lane
Alvaro Herrera writes: > 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

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

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

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-20 Thread Alvaro Herrera
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

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-16 Thread Alvaro Herrera
On 2020-Oct-16, Alvaro Herrera wrote: > I also just noticed that ALTER TABLE ONLY recurses to children, which it > should not. Apparently I wrote (bogus) bespoke code to handle recursion in EnableDisableTrigger instead of using ATSimpleRecursion. This patch seems to fix this problem. diff

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-16 Thread Alvaro Herrera
Same, with a little test. I also just noticed that ALTER TABLE ONLY recurses to children, which it should not. >From 2fb3a3122bdbbb1eb5aa6608b5132b8ab07096d4 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 16 Oct 2020 10:58:54 -0300 Subject: [PATCH] When cloning triggers, preserve

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-16 Thread Alvaro Herrera
On 2020-Sep-30, Justin Pryzby wrote: > 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, Yeah. > but CreateTrigStmt

Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-10-15 Thread Justin Pryzby
I'm hoping that Alvaro will comment on this. On Wed, Sep 30, 2020 at 05:34:50PM -0500, 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

CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers

2020-09-30 Thread Justin Pryzby
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