| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Philip Alger <paalger0(at)gmail(dot)com> |
| Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, jian he <jian(dot)universality(at)gmail(dot)com>, 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-18 15:19:30 |
| Message-ID: | 9ce36292-7b03-448c-9afd-f55af05bb51c@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 10/18/25 06:08, Philip Alger wrote:
>
> Yes, you're correct. I've added that in v6 attached.
Nice. The code now checks res for NULL, which aligns with other similar
functions, e.g. pg_get_indexdef.
if (res == NULL)
PG_RETURN_NULL();
One nitpick:
You're probably initialising the buffer a bit too early:
...
/* Validate that the relation exists */
if (!OidIsValid(relid) || get_rel_name(relid) == NULL)
PG_RETURN_NULL();
initStringInfo(&buf);
...
If the function is going to return NULL, there is no need to allocate
memory for buf. I guess you could place it right before the
appendStringInfo call:
initStringInfo(&buf);
appendStringInfo(&buf, "%s;", res);
Other than that, the patch LGTM. If the other reviewers have no
objections, I'll mark it as ready for committer.
Thanks for the patch.
Best, Jim
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David E. Wheeler | 2025-10-18 15:46:23 | Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats() |
| Previous Message | Tom Lane | 2025-10-18 15:14:49 | Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats() |