Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement - Mailing list 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@gmail.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
needto 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’),
otherwiseany 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
addsa 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/