Indicate disabled triggers in \d
Hello hackers,
I noticed that the table description given by \d <tablename> in psql
does not indicate whether a trigger is enabled or disabled.
In my opinion, if a trigger is disabled, that fact is essential
information that a person looking at the output of \d would want to
know. I would like to add this feature (and am happy to provide a
patch), and I'd like your input on how it should be displayed.
My first impulse was to just append a " (disabled)" after each
disabled trigger, but perhaps that is not visually obvious enough,
especially if the table has many triggers on it.
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
z BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff() (disabled)
You could make it more clear by putting the disabled notice on a
separate line with another level of indentation, but could look very
messy with lots of triggers on the table:
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
z BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff()
- disabled
At the moment my preference is for disabled triggers to be shown as a
separate footer section, like so:
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
Disabled triggers:
z BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff()
I think this provides the best clarity, and has the added bonus of
leaving the trigger definition intact.
Thanks for your time,
BJ
"Brendan Jurd" <direvus@gmail.com> writes:
My first impulse was to just append a " (disabled)" after each
disabled trigger, but perhaps that is not visually obvious enough,
especially if the table has many triggers on it.
Agreed, but maybe put it up at the front?
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
z (disabled) BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff()
regards, tom lane
Tom Lane wrote:
"Brendan Jurd" <direvus@gmail.com> writes:
My first impulse was to just append a " (disabled)" after each
disabled trigger, but perhaps that is not visually obvious enough,
especially if the table has many triggers on it.Agreed, but maybe put it up at the front?
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
z (disabled) BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff()
+1. This bit me recently. Or maybe, in the interests of preserving
screen space, a [*] for enabled and [x] for disabled, or something similar.
cheers
andrew
On Mon, Nov 06, 2006 at 09:12:32AM -0500, Andrew Dunstan wrote:
Tom Lane wrote:
"Brendan Jurd" <direvus@gmail.com> writes:
My first impulse was to just append a " (disabled)" after each
disabled trigger, but perhaps that is not visually obvious enough,
especially if the table has many triggers on it.Agreed, but maybe put it up at the front?
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
z (disabled) BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE
input_stuff()+1. This bit me recently. Or maybe, in the interests of preserving
screen space, a [*] for enabled and [x] for disabled, or something
similar.
For this case, I think clarity is more important than saving screen
space.
Cheers,
D
--
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778 AIM: dfetter666
Skype: davidfetter
Remember to vote!
As discussed briefly on pgsql-hackers, the current psql \d command
does not make any distinction between enabled and disabled triggers.
The attached patch modifies psql's describeOneTableDetails() such that
triggers and disabled triggers are displayed as two separate footer
lists, for example:
Triggers:
y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something()
Disabled triggers:
z BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff()
The patch compiled and tested cleanly on my machine, and passed all
regression tests.
I didn't find any relevant documentation that needed patching, so this
feature add should work fine as a standalone patch.
Regards,
BJ
Attachments:
describe.c.diffapplication/octet-stream; name=describe.c.diffDownload
Index: src/bin/psql/describe.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.147
diff -c -r1.147 describe.c
*** src/bin/psql/describe.c 9 Oct 2006 23:30:33 -0000 1.147
--- src/bin/psql/describe.c 7 Nov 2006 04:37:42 -0000
***************
*** 1054,1065 ****
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
--- 1054,1067 ----
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL,
! *result7 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
+ disabled_trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
***************
*** 1125,1131 ****
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND (not tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
--- 1127,1134 ----
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND t.tgenabled "
! "AND (NOT t.tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
***************
*** 1142,1147 ****
--- 1145,1175 ----
}
else
trigger_count = PQntuples(result4);
+
+ /* acquire disabled triggers as a separate list */
+ printfPQExpBuffer(&buf,
+ "SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
+ "FROM pg_catalog.pg_trigger t\n"
+ "WHERE t.tgrelid = '%s' "
+ "AND NOT t.tgenabled "
+ "AND (NOT t.tgisconstraint "
+ " OR NOT EXISTS"
+ " (SELECT 1 FROM pg_catalog.pg_depend d "
+ " JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
+ " WHERE d.classid = t.tableoid AND d.objid = t.oid AND d.deptype = 'i' AND c.contype = 'f'))"
+ " ORDER BY 1",
+ oid);
+ result7 = PSQLexec(buf.data, false);
+ if (!result7)
+ {
+ PQclear(result1);
+ PQclear(result2);
+ PQclear(result3);
+ PQclear(result4);
+ goto error_return;
+ }
+ else
+ disabled_trigger_count = PQntuples(result7);
}
/* count foreign-key constraints (there are none if no triggers) */
***************
*** 1160,1165 ****
--- 1188,1194 ----
PQclear(result2);
PQclear(result3);
PQclear(result4);
+ PQclear(result7);
goto error_return;
}
else
***************
*** 1305,1310 ****
--- 1334,1361 ----
}
}
+ /* print disabled triggers */
+ if (disabled_trigger_count > 0)
+ {
+ printfPQExpBuffer(&buf, _("Disabled triggers:"));
+ footers[count_footers++] = pg_strdup(buf.data);
+ for (i = 0; i < disabled_trigger_count; i++)
+ {
+ const char *tgdef;
+ const char *usingpos;
+
+ /* Everything after "TRIGGER" is echoed verbatim */
+ tgdef = PQgetvalue(result7, i, 1);
+ usingpos = strstr(tgdef, " TRIGGER ");
+ if (usingpos)
+ tgdef = usingpos + 9;
+
+ printfPQExpBuffer(&buf, " %s", tgdef);
+
+ footers[count_footers++] = pg_strdup(buf.data);
+ }
+ }
+
/* print inherits */
for (i = 0; i < inherits_count; i++)
{
On 11/7/06, Brendan Jurd <direvus@gmail.com> wrote:
As discussed briefly on pgsql-hackers, the current psql \d command
does not make any distinction between enabled and disabled triggers.The attached patch modifies psql's describeOneTableDetails() such that
triggers and disabled triggers are displayed as two separate footer
lists, for example:
Minor fix to the previous patch; result7 was not being cleared at the
end of the block.
Attachments:
describe.c.diffapplication/octet-stream; name=describe.c.diffDownload
Index: src/bin/psql/describe.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.147
diff -c -r1.147 describe.c
*** src/bin/psql/describe.c 9 Oct 2006 23:30:33 -0000 1.147
--- src/bin/psql/describe.c 7 Nov 2006 05:19:22 -0000
***************
*** 1054,1065 ****
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
--- 1054,1067 ----
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL,
! *result7 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
+ disabled_trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
***************
*** 1125,1131 ****
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND (not tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
--- 1127,1134 ----
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND t.tgenabled "
! "AND (NOT t.tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
***************
*** 1142,1147 ****
--- 1145,1175 ----
}
else
trigger_count = PQntuples(result4);
+
+ /* acquire disabled triggers as a separate list */
+ printfPQExpBuffer(&buf,
+ "SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
+ "FROM pg_catalog.pg_trigger t\n"
+ "WHERE t.tgrelid = '%s' "
+ "AND NOT t.tgenabled "
+ "AND (NOT t.tgisconstraint "
+ " OR NOT EXISTS"
+ " (SELECT 1 FROM pg_catalog.pg_depend d "
+ " JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
+ " WHERE d.classid = t.tableoid AND d.objid = t.oid AND d.deptype = 'i' AND c.contype = 'f'))"
+ " ORDER BY 1",
+ oid);
+ result7 = PSQLexec(buf.data, false);
+ if (!result7)
+ {
+ PQclear(result1);
+ PQclear(result2);
+ PQclear(result3);
+ PQclear(result4);
+ goto error_return;
+ }
+ else
+ disabled_trigger_count = PQntuples(result7);
}
/* count foreign-key constraints (there are none if no triggers) */
***************
*** 1160,1165 ****
--- 1188,1194 ----
PQclear(result2);
PQclear(result3);
PQclear(result4);
+ PQclear(result7);
goto error_return;
}
else
***************
*** 1305,1310 ****
--- 1334,1361 ----
}
}
+ /* print disabled triggers */
+ if (disabled_trigger_count > 0)
+ {
+ printfPQExpBuffer(&buf, _("Disabled triggers:"));
+ footers[count_footers++] = pg_strdup(buf.data);
+ for (i = 0; i < disabled_trigger_count; i++)
+ {
+ const char *tgdef;
+ const char *usingpos;
+
+ /* Everything after "TRIGGER" is echoed verbatim */
+ tgdef = PQgetvalue(result7, i, 1);
+ usingpos = strstr(tgdef, " TRIGGER ");
+ if (usingpos)
+ tgdef = usingpos + 9;
+
+ printfPQExpBuffer(&buf, " %s", tgdef);
+
+ footers[count_footers++] = pg_strdup(buf.data);
+ }
+ }
+
/* print inherits */
for (i = 0; i < inherits_count; i++)
{
***************
*** 1340,1345 ****
--- 1391,1397 ----
PQclear(result4);
PQclear(result5);
PQclear(result6);
+ PQclear(result7);
}
printTable(title.data, headers,
On Tue, 2006-11-07 at 16:21 +1100, Brendan Jurd wrote:
Minor fix to the previous patch; result7 was not being cleared at the
end of the block.
The patch still leaks result7 circa line 1400 (CVS HEAD). I didn't look
closely, but you probably also leak result7 circa line 1209, if result6
is NULL.
(Yeah, we definitely need to refactor describeOneTableDetails().)
-Neil
On 11/11/06, Neil Conway <neilc@samurai.com> wrote:
The patch still leaks result7 circa line 1400 (CVS HEAD). I didn't look
closely, but you probably also leak result7 circa line 1209, if result6
is NULL.
New version of the patch attached (against CVS HEAD) that fixes these
two issues.
(Yeah, we definitely need to refactor describeOneTableDetails().)
I'd be interested in doing some work on this. What did you have in mind?
BJ
Attachments:
describe_disabled_trigs.diffapplication/octet-stream; name=describe_disabled_trigs.diffDownload
Index: src/bin/psql/describe.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.148
diff -c -r1.148 describe.c
*** src/bin/psql/describe.c 8 Nov 2006 01:22:55 -0000 1.148
--- src/bin/psql/describe.c 11 Nov 2006 01:32:23 -0000
***************
*** 1054,1065 ****
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
--- 1054,1067 ----
*result3 = NULL,
*result4 = NULL,
*result5 = NULL,
! *result6 = NULL,
! *result7 = NULL;
int check_count = 0,
index_count = 0,
foreignkey_count = 0,
rule_count = 0,
trigger_count = 0,
+ disabled_trigger_count = 0,
inherits_count = 0;
int count_footers = 0;
***************
*** 1125,1131 ****
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND (not tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
--- 1127,1134 ----
"SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
"FROM pg_catalog.pg_trigger t\n"
"WHERE t.tgrelid = '%s' "
! "AND t.tgenabled "
! "AND (NOT t.tgisconstraint "
" OR NOT EXISTS"
" (SELECT 1 FROM pg_catalog.pg_depend d "
" JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
***************
*** 1142,1147 ****
--- 1145,1175 ----
}
else
trigger_count = PQntuples(result4);
+
+ /* acquire disabled triggers as a separate list */
+ printfPQExpBuffer(&buf,
+ "SELECT t.tgname, pg_catalog.pg_get_triggerdef(t.oid)\n"
+ "FROM pg_catalog.pg_trigger t\n"
+ "WHERE t.tgrelid = '%s' "
+ "AND NOT t.tgenabled "
+ "AND (NOT t.tgisconstraint "
+ " OR NOT EXISTS"
+ " (SELECT 1 FROM pg_catalog.pg_depend d "
+ " JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) "
+ " WHERE d.classid = t.tableoid AND d.objid = t.oid AND d.deptype = 'i' AND c.contype = 'f'))"
+ " ORDER BY 1",
+ oid);
+ result7 = PSQLexec(buf.data, false);
+ if (!result7)
+ {
+ PQclear(result1);
+ PQclear(result2);
+ PQclear(result3);
+ PQclear(result4);
+ goto error_return;
+ }
+ else
+ disabled_trigger_count = PQntuples(result7);
}
/* count foreign-key constraints (there are none if no triggers) */
***************
*** 1160,1165 ****
--- 1188,1194 ----
PQclear(result2);
PQclear(result3);
PQclear(result4);
+ PQclear(result7);
goto error_return;
}
else
***************
*** 1177,1182 ****
--- 1206,1212 ----
PQclear(result3);
PQclear(result4);
PQclear(result5);
+ PQclear(result7);
goto error_return;
}
else
***************
*** 1312,1317 ****
--- 1342,1369 ----
}
}
+ /* print disabled triggers */
+ if (disabled_trigger_count > 0)
+ {
+ printfPQExpBuffer(&buf, _("Disabled triggers:"));
+ footers[count_footers++] = pg_strdup(buf.data);
+ for (i = 0; i < disabled_trigger_count; i++)
+ {
+ const char *tgdef;
+ const char *usingpos;
+
+ /* Everything after "TRIGGER" is echoed verbatim */
+ tgdef = PQgetvalue(result7, i, 1);
+ usingpos = strstr(tgdef, " TRIGGER ");
+ if (usingpos)
+ tgdef = usingpos + 9;
+
+ printfPQExpBuffer(&buf, " %s", tgdef);
+
+ footers[count_footers++] = pg_strdup(buf.data);
+ }
+ }
+
/* print inherits */
for (i = 0; i < inherits_count; i++)
{
***************
*** 1347,1352 ****
--- 1399,1405 ----
PQclear(result4);
PQclear(result5);
PQclear(result6);
+ PQclear(result7);
}
printTable(title.data, headers,
This has been saved for the 8.3 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
Brendan Jurd wrote:
On 11/7/06, Brendan Jurd <direvus@gmail.com> wrote:
As discussed briefly on pgsql-hackers, the current psql \d command
does not make any distinction between enabled and disabled triggers.The attached patch modifies psql's describeOneTableDetails() such that
triggers and disabled triggers are displayed as two separate footer
lists, for example:Minor fix to the previous patch; result7 was not being cleared at the
end of the block.
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Patch applied by Neil. Thanks.
---------------------------------------------------------------------------
Brendan Jurd wrote:
On 11/7/06, Brendan Jurd <direvus@gmail.com> wrote:
As discussed briefly on pgsql-hackers, the current psql \d command
does not make any distinction between enabled and disabled triggers.The attached patch modifies psql's describeOneTableDetails() such that
triggers and disabled triggers are displayed as two separate footer
lists, for example:Minor fix to the previous patch; result7 was not being cleared at the
end of the block.
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +