Thanks Rui for the review. I have tried to fix the issues you raised. The v16 patch is ready for review/commit.
On Wed, Jul 8, 2026 at 10:28 PM Rui Zhao <[email protected]> wrote: > Hi Akshay, > > I tested v15 on current master (57f93af36f): builds, make check passes, and > basic reconstruction, pretty mode, NULL handling, and default-clause > omission > all look good. Two things. > > 1) Back in the v3/v4 discussion you decided the ON <table> name should > always > be schema-qualified for safety (the pg_get_triggerdef_worker thread with > jian he), and that pretty here only controls formatting, not schema (Phil's > point). That reasoning didn't reach the USING / WITH CHECK expressions, > though: object references inside them are still qualified only by the > caller's > search_path, so they can lose their schema and rebind to a different object > when the DDL is replayed elsewhere: > > CREATE FUNCTION s1.f(int) RETURNS bool LANGUAGE sql AS 'SELECT $1 > 0'; > CREATE FUNCTION s2.f(int) RETURNS bool LANGUAGE sql AS 'SELECT $1 < 0'; > CREATE POLICY pf ON t2 USING (s1.f(a)); > > SET search_path = public, s1; > SELECT ddl FROM pg_get_policy_ddl('t2', 'pf') AS ddl; > -- CREATE POLICY pf ON public.t2 USING (f(a)); -- s1. dropped > > SET search_path = public, s2; > CREATE POLICY pf ON t2 USING (f(a)); -- now s2.f, > opposite meaning > > So within one statement the ON clause is always qualified but the > expression > body isn't -- and as Phil noted, the pretty flag can't fix this, since > pg_get_expr qualifies by search_path visibility regardless of pretty. > > Worth settling the contract, given where this function sits. The old > pg_get_viewdef / ruledef / indexdef functions are search_path-aware and > leave > qualification to the caller (pg_dump sets search_path empty around them so > the > output is portable). The new pg_get_*_ddl functions committed so far (role, > database, tablespace) are on global, schemaless objects, so the question > never > came up -- pg_get_policy_ddl is the first of the family whose output embeds > schema-qualifiable references. Your own pg_get_table_ddl patch already hit > this and deparses under a controlled search_path (narrowed to pg_catalog), > so > the most consistent fix is to do the same here (NewGUCNestLevel + > set_config); > an empty search_path already yields the fully-qualified s1.f(a). If instead > the intent is to follow the caller's search_path, that's fine too, but it > should be documented (and SET search_path = '' noted as the way to get > portable DDL). > > 2) The doc calls the second parameter policy_name, but the actual argument > is > policyname, so the documented named-argument call fails: > > SELECT * FROM pg_get_policy_ddl("table" => 't'::regclass, > policy_name => 'p_all'); > ERROR: function pg_get_policy_ddl(table => regclass, policy_name > => unknown) does not exist > > Regards, > Rui >
v16-0001-Add-pg_get_policy_ddl-to-reconstruct-CREATE.patch
Description: Binary data
