[PATCH] Add pretty formatting to pg_get_triggerdef

Hi, Philip,

I verified your patch on my Ubuntu 24.04 and it works as expected. The "make check" passed.
Also, the code change looks good to me.

Regards,
Steven

________________________________
From: Philip Alger <paalger0@gmail.com>
Sent: Tuesday, November 04, 2025 11:36
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: [PATCH] Add pretty formatting to pg_get_triggerdef

Hello Hackers,

Currently, `pg_get_triggerdef` includes a "pretty" flag, but it does not actually format the `CREATE TRIGGER` statement in a "pretty" way. Unlike `pg_get_viewdef`, `pg_get_ruledef`, and `pg_get_indexdef`, the purpose of pretty formatting has been to remove the schema name so it can be used by the `\d` psql comment to display triggers associated with a view or table, as shown below:

postgres=# \d main_table
Table "public.main_table"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Indexes:
"main_table_a_key" UNIQUE CONSTRAINT, btree (a)
Triggers:
foofoo AFTER INSERT ON main_table FOR EACH STATEMENT EXECUTE FUNCTION trigger_func('foo_bar')
foo_bar BEFORE INSERT ON main_table FOR EACH STATEMENT EXECUTE FUNCTION trigger_func('foo_bar')
bar_fooAFTER DELETE ON main_table FOR EACH ROW WHEN ((old.a = 123)) EXECUTE FUNCTION trigger_func('foo_bar')

This patch introduces true pretty formatting to `pg_get_triggerdef`. Additionally, it creates a new function specifically for the `\d` psql command, as that command requires schema removal and a single line statement.

With this patch, when the `pretty` parameter is set to `true`, `pg_get_triggerdef` now displays a formatted output, consistent with `pg_get_viewdef`, `pg_get_ruledef`, and `pg_get_indexdef`:

postgres=# select pg_get_triggerdef(12345, true);
pg_get_triggerdef
--------------------------------------------------
CREATE TRIGGER some_trig_foobar AFTER UPDATE +
ON some_t +
FOR EACH ROW +
WHEN (NOT new.some_col) +
EXECUTE FUNCTION dummy_update_func('foobar')
(1 row)

When the `pretty` flag is `false`, the function's behavior remains unchanged from the original implementation:

postgres=# select pg_get_triggerdef(47901, false);
pg_get_triggerdef
---------------------------------------------------------------------------------------------------------------------------------------------------
CREATE TRIGGER some_trig_foobar AFTER UPDATE ON public.some_t FOR EACH ROW WHEN ((NOT new.some_col)) EXECUTE FUNCTION dummy_update_func('foobar')
(1 row)

--
Best,
Phil Alger

On Nov 4, 2025, at 11:36, Philip Alger <paalger0@gmail.com> wrote:

--
Best,
Phil Alger
<v1-0001-Add-pretty-formatting-to-pg_get_triggerdef.patch>

1
```
+/*
+ * pg_get_triggerdef_compact
+ *		Returns trigger definition in a compact, single-line format without
+ *		schema qualification designed for the psql \d command.
+ */
+Datum
+pg_get_triggerdef_compact(PG_FUNCTION_ARGS)
+{
+	Oid			trigid = PG_GETARG_OID(0);
+	char	   *res;
+
+	res = pg_get_triggerdef_worker(trigid, PRETTYFLAG_SCHEMA);
```

I think this is a mis-use of PRETTYFLAG_SCHEMA that is for printing schema-qualified names but omitting schema.

2
```
+	if (prettyFlags & PRETTYFLAG_INDENT) 
+		appendStringInfoString(&buf, "\n    ");
```

We should not hardcode 4 white-spaces, instead, we should use the const PRETTYINDENT_STD. Did you ever consider using appendContextKeyword()? Checking for an existing usage:
```
static void
get_rule_windowclause(Query *query, deparse_context *context)
{
StringInfo buf = context->buf;
const char *sep;
ListCell *l;

sep = NULL;
foreach(l, query->windowClause)
{
WindowClause *wc = (WindowClause *) lfirst(l);

if (wc->name == NULL)
continue; /* ignore anonymous windows */

if (sep == NULL)
appendContextKeyword(context, " WINDOW ",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
else
appendStringInfoString(buf, sep);

appendStringInfo(buf, "%s AS ", quote_identifier(wc->name));

get_rule_windowspec(wc, query->targetList, context);

sep = ", ";
}
}
```

3 Looks like you forgot to update pg_get_triggerdef(), when it calls pg_get_triggerdef_worker(tirgid, false), the second parameter “false” should be updated to an int flag.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Chao,

+/*

+ * pg_get_triggerdef_compact
+ *             Returns trigger definition in a compact, single-line
format without
+ *             schema qualification designed for the psql \d command.
+ */
+Datum
+pg_get_triggerdef_compact(PG_FUNCTION_ARGS)
+{
+       Oid                     trigid = PG_GETARG_OID(0);
+       char       *res;
+
+       res = pg_get_triggerdef_worker(trigid, PRETTYFLAG_SCHEMA);
```

I think this is a mis-use of PRETTYFLAG_SCHEMA that is for printing
schema-qualified names but omitting schema.

Yes, this is to omit the schema because the functions is used to print out
the triggers when using \d in psql, The current practice isn't to print out
a schema for the table/view/etc.

2

```
+       if (prettyFlags & PRETTYFLAG_INDENT)
+               appendStringInfoString(&buf, "\n    ");
```

We should not hardcode 4 white-spaces, instead, we should use the const
PRETTYINDENT_STD. Did you ever consider using appendContextKeyword()?
Checking for an existing usage:
```

Thanks for pointing this out. I refactored the code using
appendContextKeyword() and added a few tests as well in v2 (attached).

3 Looks like you forgot to update pg_get_triggerdef(), when it calls
pg_get_triggerdef_worker(tirgid, false), the second parameter “false”
should be updated to an int flag.

I fixed this to set to 0. However, false is defined as 0 in stdbool.h
#define false 0

--
Best,
Phil Alger
EDB: https://www.enterprisedb.com

On Nov 5, 2025, at 04:56, Philip Alger <paalger0@gmail.com> wrote:

Hi Chao,

+/*
+ * pg_get_triggerdef_compact
+ *             Returns trigger definition in a compact, single-line format without
+ *             schema qualification designed for the psql \d command.
+ */
+Datum
+pg_get_triggerdef_compact(PG_FUNCTION_ARGS)
+{
+       Oid                     trigid = PG_GETARG_OID(0);
+       char       *res;
+
+       res = pg_get_triggerdef_worker(trigid, PRETTYFLAG_SCHEMA);
```

I think this is a mis-use of PRETTYFLAG_SCHEMA that is for printing schema-qualified names but omitting schema.

Yes, this is to omit the schema because the functions is used to print out the triggers when using \d in psql, The current practice isn't to print out a schema for the table/view/etc.

I guess I didn’t express myself clearly. My comment was that, this function wants to omit schema, so PRETTYFLAG_SCHEMA should not be used here, because it is for adding schema.

3 Looks like you forgot to update pg_get_triggerdef(), when it calls pg_get_triggerdef_worker(tirgid, false), the second parameter “false” should be updated to an int flag.

I fixed this to set to 0. However, false is defined as 0 in stdbool.h
#define false 0

I know false is actually 0, but passing false to an int argument is misleading.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Chao,

I think this is a mis-use of PRETTYFLAG_SCHEMA that is for printing

schema-qualified names but omitting schema.

Yes, this is to omit the schema because the functions is used to print

out the triggers when using \d in psql, The current practice isn't to print
out a schema for the table/view/etc.

I guess I didn’t express myself clearly. My comment was that, this
function wants to omit schema, so PRETTYFLAG_SCHEMA should not be used
here, because it is for adding schema.

I am not sure that's right because I am using it in a similar way as what's
written in pg_get_index_worker:

appendStringInfo(&buf, "CREATE %sINDEX %s ON %s%s USING %s (",
idxrec->indisunique ? "UNIQUE " : "",
quote_identifier(NameStr(idxrelrec->relname)),
idxrelrec->relkind == RELKIND_PARTITIONED_INDEX
&& !inherits ? "ONLY " : "",
(prettyFlags & PRETTYFLAG_SCHEMA) ?
generate_relation_name(indrelid, NIL) :
generate_qualified_relation_name(indrelid),
quote_identifier(NameStr(amrec->amname)));

For that function, with `true` set you don't get the schema:

postgres=# select pg_get_indexdef(16395, 0, true);
pg_get_indexdef
--------------------------------------------------------------------
CREATE UNIQUE INDEX main_table_a_key ON main_table USING btree (a)
(1 row)

Similarly, in this patch, you get:

postgres=# \d child3
Table "public.child3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
b | integer | | |
a | text | | |
Partition of: parent FOR VALUES IN ('CCC')
Triggers:
child3_delete_trig AFTER DELETE ON child3 REFERENCING OLD TABLE AS
old_table FOR EACH STATEMENT EXECUTE FUNCTION dump_delete()

Or this without the schema as well:

postgres=# SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE
tgrelid = 'child3'::regclass AND tgname = 'child3_insert_trig';
pg_get_triggerdef
------------------------------------------------
CREATE TRIGGER child3_insert_trig AFTER INSERT+
ON child3 +
REFERENCING NEW TABLE AS new_table +
FOR EACH STATEMENT +
EXECUTE FUNCTION dump_insert()
(1 row)

Setting prettyFlags = PRETTYFLAG_SCHEMA removes the schema.

--
Best,
Phil Alger
EDB: https://www.enterprisedb.com

I am attaching v3.

I changed the name of the function that prints out the trigger using the \d
command (below) from `pg_get_triggerdef_compact` to
`pg_get_triggerdef_string`, which matches the index naming convention for a
similar function.

postgres=# \d child3

Table "public.child3"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
b | integer | | |
a | text | | |
Partition of: parent FOR VALUES IN ('CCC')
Triggers:
child3_delete_trig AFTER DELETE ON child3 REFERENCING OLD TABLE AS
old_table FOR EACH STATEMENT EXECUTE FUNCTION dump_delete()

I've cleaned up the code by removing deparse_context variables related to
appendContextKeyword that were set as NULL or NIL. All tests passed.

--
Best,
Phil Alger
EDB: https://www.enterprisedb.com

Philip Alger <paalger0@gmail.com> writes:

I am attaching v3.

This patch will cause psql's \d to fail hard against any pre-v19
server. That's not acceptable. Note the comment at the top of
describe.c:

* Support for the various \d ("describe") commands. Note that the current
* expectation is that all functions in this file will succeed when working
* with servers of versions 9.2 and up. It's okay to omit irrelevant
* information for an old server, but not to fail outright. (But failing
* against a pre-9.2 server is allowed.)

You could get around that with a conditional on the server version,
as is done elsewhere in describe.c. But I kind of wonder why we
need this new function at all. Isn't "pg_get_triggerdef(oid, false)"
close enough?

An alternative suggestion is to keep "pg_get_triggerdef(oid, true)"
doing exactly what it does today and have the newly-introduced
function be the one that has new behavior. This might be a smarter
plan anyway to avoid side-swiping other clients that are expecting
the current behavior.

regards, tom lane

Hi Tom,

This patch will cause psql's \d to fail hard against any pre-v19
server. That's not acceptable. Note the comment at the top of
describe.c:

* Support for the various \d ("describe") commands. Note that the current
* expectation is that all functions in this file will succeed when working
* with servers of versions 9.2 and up. It's okay to omit irrelevant
* information for an old server, but not to fail outright. (But failing
* against a pre-9.2 server is allowed.)

I didn't see that at the top of the file, and didn't think the new function
would break it all.

You could get around that with a conditional on the server version,
as is done elsewhere in describe.c. But I kind of wonder why we
need this new function at all. Isn't "pg_get_triggerdef(oid, false)"
close enough?

No. That works to print out the CREATE TRIGGER statement with the
schema.relation name. I am trying to make `pretty` print similar to what is
in pg_get_viewdef and pg_get_functiondef which formats the output.

An alternative suggestion is to keep "pg_get_triggerdef(oid, true)"
doing exactly what it does today and have the newly-introduced
function be the one that has new behavior. This might be a smarter
plan anyway to avoid side-swiping other clients that are expecting
the current behavior.

Thinking aloud, I would need to create a new function like
pg_get_triggerdef_worker and let it handle the formatting.

--
Best,
Phil Alger
EDB: https://www.enterprisedb.com