psql should show disabled internal triggers

Started by Andres Freundover 12 years ago15 messages
#1Andres Freund
andres@2ndquadrant.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Dimitri Fontaine
dimitri@2ndQuadrant.fr
In reply to: Andres Freund (#1)
Re: psql should show disabled internal triggers

Andres Freund <andres@2ndquadrant.com> writes:

So, how about displaying disabled internal triggers in psql?

+1

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Bernd Helmle
mailings@oopsware.de
In reply to: Andres Freund (#1)
Re: psql should show disabled internal triggers

--On 18. September 2013 13:52:29 +0200 Andres Freund
<andres@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Andres Freund
andres@2ndquadrant.com
In reply to: Bernd Helmle (#3)
Re: psql should show disabled internal triggers

On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:

--On 18. September 2013 13:52:29 +0200 Andres Freund
<andres@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Bernd Helmle
mailings@oopsware.de
In reply to: Andres Freund (#4)
Re: psql should show disabled internal triggers

--On 18. September 2013 15:19:27 +0200 Andres Freund
<andres@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6fabriziomello
fabriziomello@gmail.com
In reply to: Andres Freund (#4)
Re: psql should show disabled internal triggers

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]psql-display-all-triggers-v1.patch <http://postgresql.1045698.n5.nabble.com/file/n5775954/psql-display-all-triggers-v1.patch&gt; 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&gt;
<http://postgresql.1045698.n5.nabble.com/file/n5775954/psql-display-all-triggers-v1.patch&gt;

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: fabriziomello (#6)
1 attachment(s)
Re: psql should show disabled internal triggers

On Fri, Oct 25, 2013 at 3:37 PM, fabriziomello <fabriziomello@gmail.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

Show quoted text

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

Attachments:

psql-display-all-triggers-v2.patchtext/x-diff; charset=US-ASCII; name=psql-display-all-triggers-v2.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 96322ca..f457e21 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2079,7 +2079,7 @@ describeOneTableDetails(const char *schemaname,
 						  (pset.sversion >= 90000 ? ", true" : ""),
 						  oid);
 		if (pset.sversion >= 90000)
-			appendPQExpBufferStr(&buf, "NOT t.tgisinternal");
+			appendPQExpBuffer(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))");
 		else if (pset.sversion >= 80300)
 			appendPQExpBufferStr(&buf, "t.tgconstraint = 0");
 		else
#8Bruce Momjian
bruce@momjian.us
In reply to: Fabrízio de Royes Mello (#7)
1 attachment(s)
Re: psql should show disabled internal triggers

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@gmail.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Attachments:

trigger.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 3cb8df2..a194ce7
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 2090,2104 ****
  		printfPQExpBuffer(&buf,
  						  "SELECT t.tgname, "
  						  "pg_catalog.pg_get_triggerdef(t.oid%s), "
! 						  "t.tgenabled\n"
  						  "FROM pg_catalog.pg_trigger t\n"
  						  "WHERE t.tgrelid = '%s' AND ",
  						  (pset.sversion >= 90000 ? ", true" : ""),
! 						  oid);
  		if (pset.sversion >= 90000)
! 			appendPQExpBufferStr(&buf, "NOT t.tgisinternal");
  		else if (pset.sversion >= 80300)
! 			appendPQExpBufferStr(&buf, "t.tgconstraint = 0");
  		else
  			appendPQExpBufferStr(&buf,
  								 "(NOT tgisconstraint "
--- 2090,2108 ----
  		printfPQExpBuffer(&buf,
  						  "SELECT t.tgname, "
  						  "pg_catalog.pg_get_triggerdef(t.oid%s), "
! 						  "t.tgenabled, %s\n"
  						  "FROM pg_catalog.pg_trigger t\n"
  						  "WHERE t.tgrelid = '%s' AND ",
  						  (pset.sversion >= 90000 ? ", true" : ""),
! 						  (pset.sversion >= 90000 ? "t.tgisinternal" :
! 						   pset.sversion >= 80300 ?
! 						   "t.tgconstraint <> 0 AS tgisinternal" :
! 						   "false AS tgisinternal"), oid);
  		if (pset.sversion >= 90000)
! 			/* display/warn about disabled internal triggers */
! 			appendPQExpBuffer(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))");
  		else if (pset.sversion >= 80300)
! 			appendPQExpBufferStr(&buf, "(t.tgconstraint = 0 OR (t.tgconstraint <> 0 AND t.tgenabled = 'D'))");
  		else
  			appendPQExpBufferStr(&buf,
  								 "(NOT tgisconstraint "
*************** describeOneTableDetails(const char *sche
*** 2124,2130 ****
  			 * disabled triggers and the two special ALWAYS and REPLICA
  			 * configurations.
  			 */
! 			for (category = 0; category < 4; category++)
  			{
  				have_heading = false;
  				for (i = 0; i < tuples; i++)
--- 2128,2134 ----
  			 * disabled triggers and the two special ALWAYS and REPLICA
  			 * configurations.
  			 */
! 			for (category = 0; category <= 4; category++)
  			{
  				have_heading = false;
  				for (i = 0; i < tuples; i++)
*************** describeOneTableDetails(const char *sche
*** 2133,2143 ****
--- 2137,2149 ----
  					const char *tgdef;
  					const char *usingpos;
  					const char *tgenabled;
+ 					const char *tgisinternal;
  
  					/*
  					 * Check if this trigger falls into the current category
  					 */
  					tgenabled = PQgetvalue(result, i, 2);
+ 					tgisinternal = PQgetvalue(result, i, 3);
  					list_trigger = false;
  					switch (category)
  					{
*************** describeOneTableDetails(const char *sche
*** 2146,2159 ****
  								list_trigger = true;
  							break;
  						case 1:
! 							if (*tgenabled == 'D' || *tgenabled == 'f')
  								list_trigger = true;
  							break;
  						case 2:
! 							if (*tgenabled == 'A')
  								list_trigger = true;
  							break;
  						case 3:
  							if (*tgenabled == 'R')
  								list_trigger = true;
  							break;
--- 2152,2171 ----
  								list_trigger = true;
  							break;
  						case 1:
! 							if ((*tgenabled == 'D' || *tgenabled == 'f') &&
! 								*tgisinternal == 'f')
  								list_trigger = true;
  							break;
  						case 2:
! 							if ((*tgenabled == 'D' || *tgenabled == 'f') &&
! 								*tgisinternal == 't')
  								list_trigger = true;
  							break;
  						case 3:
+ 							if (*tgenabled == 'A')
+ 								list_trigger = true;
+ 							break;
+ 						case 4:
  							if (*tgenabled == 'R')
  								list_trigger = true;
  							break;
*************** describeOneTableDetails(const char *sche
*** 2170,2181 ****
  								printfPQExpBuffer(&buf, _("Triggers:"));
  								break;
  							case 1:
! 								printfPQExpBuffer(&buf, _("Disabled triggers:"));
  								break;
  							case 2:
! 								printfPQExpBuffer(&buf, _("Triggers firing always:"));
  								break;
  							case 3:
  								printfPQExpBuffer(&buf, _("Triggers firing on replica only:"));
  								break;
  
--- 2182,2199 ----
  								printfPQExpBuffer(&buf, _("Triggers:"));
  								break;
  							case 1:
! 								if (pset.sversion >= 80300)
! 									printfPQExpBuffer(&buf, _("Disabled user triggers:"));
! 								else
! 									printfPQExpBuffer(&buf, _("Disabled triggers:"));
  								break;
  							case 2:
! 									printfPQExpBuffer(&buf, _("Disabled internal triggers:"));
  								break;
  							case 3:
+ 								printfPQExpBuffer(&buf, _("Triggers firing always:"));
+ 								break;
+ 							case 4:
  								printfPQExpBuffer(&buf, _("Triggers firing on replica only:"));
  								break;
  
#9Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Bruce Momjian (#8)
Re: psql should show disabled internal triggers

On Thu, Feb 13, 2014 at 12:04 AM, Bruce Momjian <bruce@momjian.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@gmail.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

Show quoted text

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

#10Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#8)
Re: psql should show disabled internal triggers

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Andres Freund
andres@2ndquadrant.com
In reply to: Bruce Momjian (#10)
Re: psql should show disabled internal triggers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Bruce Momjian
bruce@momjian.us
In reply to: Andres Freund (#11)
Re: psql should show disabled internal triggers

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Andres Freund
andres@2ndquadrant.com
In reply to: Bruce Momjian (#12)
Re: psql should show disabled internal triggers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Andres Freund (#13)
Re: psql should show disabled internal triggers

On Mon, Feb 24, 2014 at 3:23 PM, Andres Freund <andres@2ndquadrant.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

Show quoted text

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

#15Bruce Momjian
bruce@momjian.us
In reply to: Andres Freund (#13)
Re: psql should show disabled internal triggers

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers