Re: psql should show disabled internal triggers

Hi,

If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
re-enable the disabled triggers it's easy to miss internal triggers.
A \d+ tablename will not show anything out of the ordinary for that
situation since we don't show internal triggers. But foreign key checks
won't work.
So, how about displaying disabled internal triggers in psql?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> So, how about displaying disabled internal triggers in psql?

+1

--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--On 18. September 2013 13:52:29 +0200 Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

> If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> re-enable the disabled triggers it's easy to miss internal triggers.
> A \d+ tablename will not show anything out of the ordinary for that
> situation since we don't show internal triggers. But foreign key checks
> won't work.
> So, how about displaying disabled internal triggers in psql?

Hi had exactly the same concerns this morning while starting to look at the
ENABLE/DISABLE constraint patch. However, i wouldn't display them as
triggers, but maybe more generally as "disabled constraints" or such.

--
Thanks

Bernd

On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
>
>
> --On 18. September 2013 13:52:29 +0200 Andres Freund
> <andres(at)2ndquadrant(dot)com> wrote:
>
> >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> >re-enable the disabled triggers it's easy to miss internal triggers.
> >A \d+ tablename will not show anything out of the ordinary for that
> >situation since we don't show internal triggers. But foreign key checks
> >won't work.
> >So, how about displaying disabled internal triggers in psql?
>
> Hi had exactly the same concerns this morning while starting to look at the
> ENABLE/DISABLE constraint patch. However, i wouldn't display them as
> triggers, but maybe more generally as "disabled constraints" or such.

Well, that will lead the user in the wrong direction, won't it? They
haven't disabled the constraint but the trigger. Especially as we
already have NOT VALID and might grow DISABLED for constraint
themselves...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--On 18. September 2013 15:19:27 +0200 Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

> Well, that will lead the user in the wrong direction, won't it? They
> haven't disabled the constraint but the trigger. Especially as we
> already have NOT VALID and might grow DISABLED for constraint
> themselves...
>

Valid point. But it is also nice to know in detail, which constraints
stopped working. Ok, it is documented which constraints are affected and
maybe i'm lost within too much detail atm, but i find people getting
confused about this internal trigger thingie sometimes. Won't they get
confused about a suddenly appearing RI_ConstraintTrigger_a_54015, too?

--
Thanks

Bernd

On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > --On 18. September 2013 13:52:29 +0200 Andres Freund
> > &lt;andres@&gt; wrote:
> >
> > >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> > >re-enable the disabled triggers it's easy to miss internal triggers.
> > >A \d+ tablename will not show anything out of the ordinary for that
> > >situation since we don't show internal triggers. But foreign key checks
> > >won't work.
> > >So, how about displaying disabled internal triggers in psql?
> >
> > Hi had exactly the same concerns this morning while starting to look at
> the
> > ENABLE/DISABLE constraint patch. However, i wouldn't display them as
> > triggers, but maybe more generally as "disabled constraints" or such.
>
> Well, that will lead the user in the wrong direction, won't it? They
> haven't disabled the constraint but the trigger. Especially as we
> already have NOT VALID and might grow DISABLED for constraint
> themselves...
>

Hi,

The attached patch [1] enable PSQL to list internal disabled triggers in \d
only in
versions >= 9.0.

[1] psql-display-all-triggers-v1.patch
<http://postgresql.1045698.n5.nabble.com/file/n5775954/psql-display-all-triggers-v1.patch>

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

-----
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
--
View this message in context: http://postgresql.1045698.n5.nabble.com/psql-should-show-disabled-internal-triggers-tp5771406p5775954.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

On Fri, Oct 25, 2013 at 3:37 PM, fabriziomello <fabriziomello(at)gmail(dot)com>
wrote:
>
> On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > --On 18. September 2013 13:52:29 +0200 Andres Freund
> > > &lt;andres@&gt; wrote:
> > >
> > > >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> > > >re-enable the disabled triggers it's easy to miss internal triggers.
> > > >A \d+ tablename will not show anything out of the ordinary for that
> > > >situation since we don't show internal triggers. But foreign key
checks
> > > >won't work.
> > > >So, how about displaying disabled internal triggers in psql?
> > >
> > > Hi had exactly the same concerns this morning while starting to look
at
> > the
> > > ENABLE/DISABLE constraint patch. However, i wouldn't display them as
> > > triggers, but maybe more generally as "disabled constraints" or such.
> >
> > Well, that will lead the user in the wrong direction, won't it? They
> > haven't disabled the constraint but the trigger. Especially as we
> > already have NOT VALID and might grow DISABLED for constraint
> > themselves...
> >
>
> Hi,
>
> The attached patch [1] enable PSQL to list internal disabled triggers in
\d
> only in versions >= 9.0.
>
> [1] psql-display-all-triggers-v1.patch
> <
http://postgresql.1045698.n5.nabble.com/file/n5775954/psql-display-all-triggers-v1.patch
>
>

Hi all,

I'm just send a new WIP patch rebased from master.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Thu, Nov 21, 2013 at 11:59:51PM -0200, Fabrízio de Royes Mello wrote:
> On Fri, Oct 25, 2013 at 3:37 PM, fabriziomello <fabriziomello(at)gmail(dot)com> wrote:
> >
> > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > > --On 18. September 2013 13:52:29 +0200 Andres Freund
> > > > &lt;andres@&gt; wrote:
> > > >
> > > > >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> > > > >re-enable the disabled triggers it's easy to miss internal triggers.
> > > > >A \d+ tablename will not show anything out of the ordinary for that
> > > > >situation since we don't show internal triggers. But foreign key checks
> > > > >won't work.
> > > > >So, how about displaying disabled internal triggers in psql?
> > > >
> > > > Hi had exactly the same concerns this morning while starting to look at
> > > the
> > > > ENABLE/DISABLE constraint patch. However, i wouldn't display them as
> > > > triggers, but maybe more generally as "disabled constraints" or such.
> > >
> > > Well, that will lead the user in the wrong direction, won't it? They
> > > haven't disabled the constraint but the trigger. Especially as we
> > > already have NOT VALID and might grow DISABLED for constraint
> > > themselves...
> > >
> >
> > Hi,
> >
> > The attached patch [1] enable PSQL to list internal disabled triggers in \d
> > only in versions >= 9.0.
> >
> > [1] psql-display-all-triggers-v1.patch
> > <http://postgresql.1045698.n5.nabble.com/file/n5775954/
> psql-display-all-triggers-v1.patch>

As others, I am concerned about people being confused when funny-looking
trigger names suddenly appearing when you disable all table triggers.

What I ended up doing is to create a user and internal section when
displaying disabled triggers:

Disabled user triggers:
check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
Disabled internal triggers:
"RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
"RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...

I kept the "Triggers" section unchanged, showing only user triggers. I
also updated the code to handle 8.3+ servers.

Patch attached.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Thu, Feb 13, 2014 at 12:04 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Thu, Nov 21, 2013 at 11:59:51PM -0200, Fabrízio de Royes Mello wrote:
> > On Fri, Oct 25, 2013 at 3:37 PM, fabriziomello <fabriziomello(at)gmail(dot)com>
wrote:
> > >
> > > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > > > --On 18. September 2013 13:52:29 +0200 Andres Freund
> > > > > &lt;andres@&gt; wrote:
> > > > >
> > > > > >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then
individually
> > > > > >re-enable the disabled triggers it's easy to miss internal
triggers.
> > > > > >A \d+ tablename will not show anything out of the ordinary for
that
> > > > > >situation since we don't show internal triggers. But foreign key
checks
> > > > > >won't work.
> > > > > >So, how about displaying disabled internal triggers in psql?
> > > > >
> > > > > Hi had exactly the same concerns this morning while starting to
look at
> > > > the
> > > > > ENABLE/DISABLE constraint patch. However, i wouldn't display them
as
> > > > > triggers, but maybe more generally as "disabled constraints" or
such.
> > > >
> > > > Well, that will lead the user in the wrong direction, won't it? They
> > > > haven't disabled the constraint but the trigger. Especially as we
> > > > already have NOT VALID and might grow DISABLED for constraint
> > > > themselves...
> > > >
> > >
> > > Hi,
> > >
> > > The attached patch [1] enable PSQL to list internal disabled triggers
in \d
> > > only in versions >= 9.0.
> > >
> > > [1] psql-display-all-triggers-v1.patch
> > > <http://postgresql.1045698.n5.nabble.com/file/n5775954/
> > psql-display-all-triggers-v1.patch>
>
> As others, I am concerned about people being confused when funny-looking
> trigger names suddenly appearing when you disable all table triggers.
>
> What I ended up doing is to create a user and internal section when
> displaying disabled triggers:
>
> Disabled user triggers:
> check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE
PROCEDURE trigf()
> Disabled internal triggers:
> "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM
customer NOT DEF ...
> "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM
customer NOT DEF ...
>
> I kept the "Triggers" section unchanged, showing only user triggers. I
> also updated the code to handle 8.3+ servers.
>
> Patch attached.
>

Makes more sense than my previous patch...

The code looks fine to me!!

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Wed, Feb 12, 2014 at 09:04:45PM -0500, Bruce Momjian wrote:
> As others, I am concerned about people being confused when funny-looking
> trigger names suddenly appearing when you disable all table triggers.
>
> What I ended up doing is to create a user and internal section when
> displaying disabled triggers:
>
> Disabled user triggers:
> check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
> Disabled internal triggers:
> "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
> "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...
>
> I kept the "Triggers" section unchanged, showing only user triggers. I
> also updated the code to handle 8.3+ servers.
>
> Patch attached.

Patch applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 2014-02-24 12:45:12 -0500, Bruce Momjian wrote:
> On Wed, Feb 12, 2014 at 09:04:45PM -0500, Bruce Momjian wrote:
> > As others, I am concerned about people being confused when funny-looking
> > trigger names suddenly appearing when you disable all table triggers.
> >
> > What I ended up doing is to create a user and internal section when
> > displaying disabled triggers:
> >
> > Disabled user triggers:
> > check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
> > Disabled internal triggers:
> > "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
> > "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...
> >
> > I kept the "Triggers" section unchanged, showing only user triggers. I
> > also updated the code to handle 8.3+ servers.
> >
> > Patch attached.
>
> Patch applied.

Thanks. It'd have been nice tho, to mention Fabrízio in the commit
message as the patch's author.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Feb 24, 2014 at 07:09:29PM +0100, Andres Freund wrote:
> On 2014-02-24 12:45:12 -0500, Bruce Momjian wrote:
> > On Wed, Feb 12, 2014 at 09:04:45PM -0500, Bruce Momjian wrote:
> > > As others, I am concerned about people being confused when funny-looking
> > > trigger names suddenly appearing when you disable all table triggers.
> > >
> > > What I ended up doing is to create a user and internal section when
> > > displaying disabled triggers:
> > >
> > > Disabled user triggers:
> > > check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
> > > Disabled internal triggers:
> > > "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
> > > "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...
> > >
> > > I kept the "Triggers" section unchanged, showing only user triggers. I
> > > also updated the code to handle 8.3+ servers.
> > >
> > > Patch attached.
> >
> > Patch applied.
>
> Thanks. It'd have been nice tho, to mention Fabrízio in the commit
> message as the patch's author.

Uh, I was thinking of that, but I basically rewrote the patch from
scratch and changed its visible behavior, so I was worried about perhaps
blaming him if it introduced a bug. I should have said "original patch
by ...", but because so much of it was new, I didn't bother.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 2014-02-24 13:16:39 -0500, Bruce Momjian wrote:
> On Mon, Feb 24, 2014 at 07:09:29PM +0100, Andres Freund wrote:
> > On 2014-02-24 12:45:12 -0500, Bruce Momjian wrote:
> > > On Wed, Feb 12, 2014 at 09:04:45PM -0500, Bruce Momjian wrote:
> > > > As others, I am concerned about people being confused when funny-looking
> > > > trigger names suddenly appearing when you disable all table triggers.
> > > >
> > > > What I ended up doing is to create a user and internal section when
> > > > displaying disabled triggers:
> > > >
> > > > Disabled user triggers:
> > > > check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
> > > > Disabled internal triggers:
> > > > "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
> > > > "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...
> > > >
> > > > I kept the "Triggers" section unchanged, showing only user triggers. I
> > > > also updated the code to handle 8.3+ servers.
> > > >
> > > > Patch attached.
> > >
> > > Patch applied.
> >
> > Thanks. It'd have been nice tho, to mention Fabrízio in the commit
> > message as the patch's author.
>
> Uh, I was thinking of that, but I basically rewrote the patch from
> scratch and changed its visible behavior, so I was worried about perhaps
> blaming him if it introduced a bug. I should have said "original patch
> by ...", but because so much of it was new, I didn't bother.

I just seems nicer to relatively new contributors to mention their names
when they try to contribute.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Feb 24, 2014 at 3:23 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:
> > > Thanks. It'd have been nice tho, to mention Fabrízio in the commit
> > > message as the patch's author.
> >
> > Uh, I was thinking of that, but I basically rewrote the patch from
> > scratch and changed its visible behavior, so I was worried about perhaps
> > blaming him if it introduced a bug. I should have said "original patch
> > by ...", but because so much of it was new, I didn't bother.
>
> I just seems nicer to relatively new contributors to mention their names
> when they try to contribute.
>

Hey guys, I'm not worried about it... to me the most important thing is the
improvement and the learning. So I'm happy to help in some way.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Mon, Feb 24, 2014 at 07:23:50PM +0100, Andres Freund wrote:
> On 2014-02-24 13:16:39 -0500, Bruce Momjian wrote:
> > On Mon, Feb 24, 2014 at 07:09:29PM +0100, Andres Freund wrote:
> > > On 2014-02-24 12:45:12 -0500, Bruce Momjian wrote:
> > > > On Wed, Feb 12, 2014 at 09:04:45PM -0500, Bruce Momjian wrote:
> > > > > As others, I am concerned about people being confused when funny-looking
> > > > > trigger names suddenly appearing when you disable all table triggers.
> > > > >
> > > > > What I ended up doing is to create a user and internal section when
> > > > > displaying disabled triggers:
> > > > >
> > > > > Disabled user triggers:
> > > > > check_update BEFORE UPDATE ON orders FOR EACH ROW EXECUTE PROCEDURE trigf()
> > > > > Disabled internal triggers:
> > > > > "RI_ConstraintTrigger_c_16409" AFTER INSERT ON orders FROM customer NOT DEF ...
> > > > > "RI_ConstraintTrigger_c_16410" AFTER UPDATE ON orders FROM customer NOT DEF ...
> > > > >
> > > > > I kept the "Triggers" section unchanged, showing only user triggers. I
> > > > > also updated the code to handle 8.3+ servers.
> > > > >
> > > > > Patch attached.
> > > >
> > > > Patch applied.
> > >
> > > Thanks. It'd have been nice tho, to mention Fabrízio in the commit
> > > message as the patch's author.
> >
> > Uh, I was thinking of that, but I basically rewrote the patch from
> > scratch and changed its visible behavior, so I was worried about perhaps
> > blaming him if it introduced a bug. I should have said "original patch
> > by ...", but because so much of it was new, I didn't bother.
>
> I just seems nicer to relatively new contributors to mention their names
> when they try to contribute.

Agreed.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +