Analysis: pg_get_trigger_ddl() and pg_get_policy_ddl() — Retail DDL Functions
Architectural Context
This thread is part of the broader "Retail DDL Functions" project initiated by Andrew Dunstan (referenced at 945db7c5-be75-45bf-b55b-cb1e56f2e3e9@dunslane.net). The goal is to expose per-object DDL reconstruction as first-class SQL-callable functions, building on the existing pg_get_*def() family (pg_get_viewdef, pg_get_indexdef, pg_get_constraintdef, pg_get_triggerdef, pg_get_functiondef). The motivation is to make object-level DDL retrieval practical without pg_dump — useful for schema cloning, multi-tenant sync, ORM tooling, and inspection.
Two parallel sub-patches ended up intertwined in this thread:
pg_get_trigger_ddl(regclass, name)by Philip Alger — retrievesCREATE TRIGGERtext given a table and trigger name instead of OID.pg_get_policy_ddl(regclass, name, bool pretty)by Akshay Joshi (EDB) — reconstructsCREATE POLICYfrompg_policy, with a pretty-print mode.
Core Technical Problems
1. Identifier-to-OID lookup ergonomics
pg_get_triggerdef(oid) requires the caller to find the trigger OID first — non-trivial when multiple tables share trigger names. Philip's function sidesteps this by accepting (regclass, name). The regclass cast piggybacks on PostgreSQL's existing identifier resolution machinery (search_path, schema-qualification, quoting).
Cary Huang argued that adding pg_get_trigger_ddl fragments the API and instead favored overloading pg_get_triggerdef itself with a (name, name) signature. The counter-argument (and the prevailing direction) was that pg_get_viewdef(text) — the historical precedent for name-based lookup — is now deprecated in favor of OID/regclass variants. Álvaro Herrera confirmed that the pg_get_viewdef(text) arrangement predates regclass (commit 52200befd0) and "we wouldn't do it this way nowadays". The regclass approach is the modern idiom.
2. Quoted / case-sensitive identifier handling
This was the most substantive technical dispute. The function takes trigger names as text, not name, which created ambiguity:
- Philip's initial approach: accept the literal text as-is, matching case-sensitively. This broke for users who typed
'"Foo"'(with quotes) — a natural instinct given thatregclassaccepts quoted forms. - Jim Jones's objection: inconsistency. The first arg (
regclass) requires quoting for case-sensitive names, so the second arg should too. He citedpg_get_viewdef('"MyView"')behavior. - v5 resolution: Philip used
textToQualifiedNameList()+DeconstructQualifiedName()to downcase-fold unquoted names and strip quotes from quoted names — mirroring SQL identifier rules. - Late regression (Soumya Murali, 2026-05): this broke names containing special characters like
"Weird-Trigger!"that would never parse as a valid qualified name.textToQualifiedNameListapplies identifier parsing which lowercases and rejects punctuation. Soumya's proposed fix was to revert toobjName = text_to_cstring(trgName)— accept exact text — essentially undoing the Jim Jones concession.
This is a genuine design tension with no clean answer: identifier semantics vs. string semantics. The regclass cast resolves the first argument via the parser, so the second argument has no syntactic context to imitate. Either choice surprises some users.
3. Schema-qualified trigger names
Per the CREATE TRIGGER docs, trigger names cannot be schema-qualified (they inherit the table's schema). Jian He pointed out that v7 silently dropped the schema prefix when users passed 'public.my_trigger'. v8 added an explicit error. Jim Jones argued this extra check is unnecessary — the downstream "trigger does not exist" error would suffice — but Philip kept it for clearer user feedback.
4. Invalid OID handling
pg_get_trigger_ddl(-1, 'h') initially produced ERROR: relation with OID 4294967295 does not exist (the signed-to-unsigned wraparound). Jian He flagged this as a user-facing leak of internal representation. Precedent from pg_basetype (referenced 3759807.1711658868@sss.pgh.pa.us) suggested returning NULL for invalid input. Resolution: if (!OidIsValid(relid) || get_rel_name(relid) == NULL) PG_RETURN_NULL();.
5. Pretty-printing semantics collision
The word pretty is already overloaded in ruleutils.c via PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA — notably, pretty=true in existing functions omits schema qualification (for human readability). Akshay's pg_get_policy_ddl used pretty for multi-line formatting (\t, \n per clause) — a different semantic axis.
Akshay introduced a new GET_DDL_PRETTY_FLAGS macro to separate these concerns, because the existing GET_PRETTY_FLAGS indents unconditionally regardless of the flag's value — incorrect behavior for DDL generation where the unformatted output should be single-line. Philip noted the naming confusion and suggested format or pretty_format would be clearer.
For trigger DDL, Philip proposed a separate pg_get_triggerdef_worker_formatted(oid) to avoid breaking \d output in psql, which relies on single-line trigger definitions from pg_get_triggerdef.
6. Policy reconstruction completeness
Jian He identified two correctness issues in pg_get_policy_ddl:
- Subquery expressions in USING/WITH CHECK:
pg_get_expr()explicitly does not handle sublinks/subqueries (see commit6867f96andcheckExprHasSubLink). Row-security policy quals can contain subqueries, sopg_get_exprwill error with "expression contains variables". This is a fundamental limitation —pg_get_exprwas designed for partial index predicates and defaults with a single relation context, not arbitrary policy expressions. Proper reconstruction needs deparsing logic closer toget_query_def. - Missing
TO PUBLIC: whenpolrolescontains only the public role OID (0), the output omitted theTOclause. Fixed in v4 by defaulting to PUBLIC.
Marcos raised the ALTER TABLE ... ENABLE ROW LEVEL SECURITY question — policy DDL is incomplete without it, but Akshay correctly scoped that to table-level DDL reconstruction (a separate patch).
7. Resource hygiene nitpicks
- Table open/close lock levels: Philip caught Akshay using
NoLockforrelation_open, which violates the locking protocol (should beAccessShareLockfor read). appendStringInfo(buf, "%s;", res)vsappendStringInfoCharfor single chars — cheaper API.pfree(res)debate: Josef Šimánek noted no Valgrind leak, but Chao Li suggested explicit pfree. Philip pointed outstring_to_text()already pfrees its input — standard ruleutils pattern.- Buffer initialization order: Jim noted
initStringInfo(&buf)shouldn't happen before the null-return path.
Design Decisions & Their Weight
| Decision | Resolution | Driving Voice |
|---|---|---|
Use regclass not name for table arg |
Accepted | Álvaro Herrera (committer) |
New function name pg_get_trigger_ddl vs overload pg_get_triggerdef |
New function | Andrew Dunstan (committer, project originator) |
| Parse trigger name as identifier (downcase unquoted) | Contested, likely to revert | Soumya Murali's late feedback |
| Return NULL for invalid OID | Accepted | Jian He, following pg_basetype precedent |
| Explicit error for schema-qualified trigger name | Accepted (Philip) but challenged (Jim) | Undecided |
| Separate pretty-flag semantics via new macro | Accepted | Akshay Joshi |
| Trailing semicolon in output | Accepted | Philip (deviates from pg_get_triggerdef) |
Álvaro's and Andrew's endorsements carry substantial weight as committers with deep ruleutils.c familiarity. Jim Jones and Jian He provided the most rigorous review on edge cases. Tom Lane appeared only to answer a CI process question, not on substance.
Architectural Implications
-
Code location migration: Philip noted (2026-05) that these DDL functions "now live in ddlutils.c" — a new module splitting DDL-generation functions away from ruleutils.c's rule/view rewriting. This is a significant organizational change suggesting the Retail DDL project is being consolidated into its own subsystem.
-
pg_get_exprinsufficiency for policy deparse: The policy patch exposes thatpg_get_expris an inadequate tool for reconstructing policy DDL. Either policies need a dedicated deparser (mirroringpg_get_ruledef_worker's approach), orpg_get_exprneeds sublink support. Jian He's own related patch (commitfest 6054) confronts the same issue. -
Semantic namespace for "pretty": Adding
GET_DDL_PRETTY_FLAGSis a minor fork in ruleutils conventions. If the Retail DDL project adds 10+ newpg_get_*_ddlfunctions, this pretty semantics needs to be consistent across all of them. -
psql
\dcoupling:pg_get_triggerdef's single-line output is load-bearing for psql's\doutput. Any pretty-printing refactor must preserve the existing single-line path, motivating Philip's proposed_worker_formattedsplit rather than adding a parameter to the existing worker.
Unresolved Questions
- Permission check: Chao Li asked whether
has_table_privilege(relid, 'USAGE')should gate access. Currently any user can retrieve DDL for any trigger. This is consistent withpg_get_triggerdef(no ACL check) but may be reconsidered for the broader Retail DDL effort. - Identifier parsing for trigger name: Soumya's May 2026 feedback may force a revert to raw-text handling, which would re-open Jim's original consistency objection.
- Pretty-print scope for triggers: Philip is still developing the pretty-print path via a separate
_formattedworker.