Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Philip Alger <paalger0(at)gmail(dot)com>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement
Date: 2025-10-28 02:50:12
Message-ID: 18E4029A-E549-4433-B28D-7F0CA8B19425@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Philip,

Thanks for the patch. I just reviewed it and got a few comments:

> On Oct 23, 2025, at 06:27, Philip Alger <paalger0(at)gmail(dot)com> wrote:
>
> <v8-0001-Add-pg_get_trigger_ddl-function.patch>

1
```
+ /* Parse the trigger name to handle quoted identifiers */
+ nameList = textToQualifiedNameList(trgName);
+ if (list_length(nameList) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("trigger name cannot be schema qualified")));
+
+ DeconstructQualifiedName(nameList, &schemaName, &objName);
```

As we have checked list length must be 1, so that schemaName must be NULL and objName must be trgName, thus it doesn’t need to call DeconstructQualifiedName(), and the local variable schemaName is not needed, we can just do:

```
objName = text_to_cstring(trgName);
```

2
```
+ appendStringInfo(&buf, "%s;", res);
```

As we are only appending a single char “;”, appendStringInfoChar() is cheaper.

3
```
+ initStringInfo(&buf);
+
+ appendStringInfo(&buf, "%s;", res);
```

I think it’s better to pfree(res).

4. I am just thinking that if this function need to check permissions like has_table_privilege(relid, 'USAGE’), otherwise any user can query the DDL of triggers on other users’ tables.

5. I wonder why this function is needed as there is pg_get_triggerdef() already, only difference is that this function adds a tailing “;”.

6. I wonder if this function needs to add a third argument for pretty flag.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-10-28 03:06:03 Re: minor error message enhance: print RLS policy name when only one permissive policy exists
Previous Message Thomas Munro 2025-10-28 02:40:16 Re: C11: should we use char32_t for unicode code points?