Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement

All,

I've updated the patch to align with the interface change introduced by
commit *d6ed87d1989* (Andrew Dunstan, 2026-06-26 "Use named boolean
parameters for pg_get_*_ddl option arguments").

That commit replaced the VARIADIC text[] alternating key/value option
interface with typed named boolean parameters across pg_get_role_ddl(),
pg_get_tablespace_ddl(), and pg_get_database_ddl(), removing the
DdlOption/parse_ddl_options() machinery in favour of direct
PG_GETARG_BOOL() calls.

Updated patch v14 is ready for review/commit.

On Mon, Jun 29, 2026 at 7:12 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Rebased *v13* patch is now ready for review and commit.
>
> On Mon, Jun 22, 2026 at 6:49 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> *Chao Li* found an issue in my other patch for *pg_get_table_ddl.* Specifically,
>> the existing pattern in pg_proc.dat for variadic-text functions (e.g.,
>> jsonb_delete, json_extract_path) uses *_text* instead of *text* at the
>> variadic position in both proargtypes and proallargtypes, with provariadic
>> => 'text'. This is the convention documented by the sanity check in
>> src/test/regress/sql/opr_sanity.sql.
>>
>> I realized the same issue was present in this patch as well, so I have
>> fixed it and added corresponding test cases.
>>
>> The v12 patch is now ready for review and commit.
>>
>> On Mon, Jun 22, 2026 at 6:34 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Mon, Jun 8, 2026 at 6:10 PM solai v <solai(dot)cdac(at)gmail(dot)com> wrote:
>>>
>>>> Hi all,
>>>>
>>>>
>>>> On Mon, Jun 8, 2026 at 5:32 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>>>> >
>>>> > On Fri, 29 May 2026 at 12:20, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>> > > Thanks for the reviews.
>>>> > >
>>>> > > My original patch (v9) was actually correct. After considering
>>>> Japin's review comment, I initially thought the extra
>>>> > > parentheses weren't necessary, but they are indeed required for
>>>> handling boolean values properly in non-pretty mode too,
>>>> > > so I kept them in USING (%s) / WITH CHECK (%s) for both modes.
>>>> > >
>>>> >
>>>> > My bad! I had not considered this situation.
>>>> >
>>>> > > `pg_get_expr()` only adds outer parentheses for composite
>>>> expressions (via the deparsers for `OpExpr`, `BoolExpr`, etc.).
>>>> > > For atomic top-level nodes like `Const`, `Var`, `current_user`,
>>>> `NULL`, etc.
>>>> > > For example:
>>>> > >
>>>> > > CREATE POLICY p ON t USING (true);
>>>> > > SELECT pg_get_policy_ddl('t', 'p'); -- previously: ... USING
>>>> true; (syntax error)
>>>> > >
>>>> > > This is exactly why `pg_dump` always wraps the expression
>>>> unconditionally; see `src/bin/pg_dump/pg_dump.c`:4473-4477:
>>>> > >
>>>> > > if (polinfo->polqual != NULL)
>>>> > > appendPQExpBuffer(query, " USING (%s)", polinfo->polqual);
>>>> > > if (polinfo->polwithcheck != NULL)
>>>> > > appendPQExpBuffer(query, " WITH CHECK (%s)",
>>>> polinfo->polwithcheck);
>>>> > >
>>>> > > I've also added a round-trip regression test with `USING (true)` /
>>>> `WITH CHECK (false)` that captures the generated DDL,
>>>> > > drops the policies, re-executes the DDL, and verifies the policies
>>>> are recreated.
>>>> > >
>>>> > > v11 Patch attached for review.
>>>> > >
>>>> > > On Thu, May 28, 2026 at 7:12 PM Ilmar Y <tanswis42(at)gmail(dot)com>
>>>> wrote:
>>>> > >
>>>> > > The following review has been posted through the commitfest
>>>> application:
>>>> > > make installcheck-world: not tested
>>>> > > Implements feature: tested, failed
>>>> > > Spec compliant: not tested
>>>> > > Documentation: not tested
>>>> > >
>>>> > > Hi,
>>>> > >
>>>> > > I looked at v10, focused on whether the generated CREATE POLICY
>>>> statement
>>>> > > can be executed again.
>>>> > >
>>>> > > The patch applies cleanly on current master at
>>>> > > 8a86aa313a714adc56c74e4b08793e4e6102b5ca.
>>>> > >
>>>> > > git diff --check reports no issues.
>>>> > >
>>>> > > I built with:
>>>> > >
>>>> > > ./configure --prefix="$PWD/pg-install" --without-readline
>>>> --without-zlib --without-icu
>>>> > > make -s -j8
>>>> > > make -s install
>>>> > >
>>>> > > make -C src/test/regress check TESTS=rowsecurity
>>>> > >
>>>> > > ended up running the full parallel_schedule in this makefile; all
>>>> 245 tests
>>>> > > passed, including rowsecurity.
>>>> > >
>>>> > > I found one correctness issue in the generated non-pretty DDL.
>>>> The code
>>>> > > assumes that pg_get_expr_ext(..., false) already returns the
>>>> parentheses
>>>> > > required by CREATE POLICY syntax, but that is not true for simple
>>>> boolean
>>>> > > constants.
>>>> > >
>>>> > > For example:
>>>> > >
>>>> > > CREATE TABLE t(a int);
>>>> > > CREATE POLICY p_true ON t USING (true);
>>>> > > SELECT ddl FROM pg_get_policy_ddl('t', 'p_true', 'pretty',
>>>> 'false') AS ddl;
>>>> > >
>>>> > > returns:
>>>> > >
>>>> > > CREATE POLICY p_true ON public.t USING true;
>>>> > >
>>>> > > If I drop the policy and execute that generated statement, it
>>>> fails:
>>>> > >
>>>> > > ERROR: syntax error at or near "true"
>>>> > > LINE 1: CREATE POLICY p_true ON public.t USING true;
>>>> > > ^
>>>> > >
>>>> > > The same issue reproduces for WITH CHECK:
>>>> > >
>>>> > > CREATE POLICY p_check ON t FOR INSERT WITH CHECK (false);
>>>> > >
>>>> > > is reconstructed as:
>>>> > >
>>>> > > CREATE POLICY p_check ON public.t FOR INSERT WITH CHECK false;
>>>> > >
>>>> > > and executing it fails at "false".
>>>> > >
>>>> > > So I think USING and WITH CHECK need to be parenthesized in
>>>> non-pretty mode
>>>> > > too, or the tests should include a round-trip execution check for
>>>> generated
>>>> > > DDL with simple boolean expressions.
>>>> > >
>>>> > > I used two small SQL reproducers for the manual checks; the
>>>> complete repro is
>>>> > > included above.
>>>> > >
>>>> > > I have not reviewed the broader pg_get_*_ddl API design or every
>>>> possible
>>>> > > policy expression form.
>>>> > >
>>>> > > Regards,
>>>> > > Ilmar Yunusov
>>>> > >
>>>> > > The new status of this patch is: Waiting on Author
>>>> >
>>>>
>>>>
>>>> I reviewed and tested the V11 pg_get_policy_ddl() patch. I verified
>>>> the basic functionality by creating various row-level security
>>>> policies and checking the reconstructed DDL returned by
>>>> pg_get_policy_ddl(). The function correctly reconstructed policies for
>>>> different command types (SELECT, INSERT, UPDATE, DELETE), PERMISSIVE
>>>> and RESTRICTIVE policies, multiple role lists, quoted identifiers,
>>>> USING clauses, and WITH CHECK clauses.
>>>> I also tested more complex cases involving subqueries. Policies
>>>> containing EXISTS subqueries in both USING and WITH CHECK clauses were
>>>> reconstructed successfully. Nested subqueries were also handled
>>>> correctly, and I did not encounter any issues related to expression
>>>> reconstruction or variable handling. I verified NULL input handling as
>>>> well. Passing NULL values for the table name or policy name returned
>>>> no rows as expected. Invalid relation names resulted in the expected
>>>> regclass resolution error. One observation from testing is that the
>>>> generated DDL omits default clauses such as FOR ALL and TO PUBLIC. For
>>>> example, a policy created with FOR ALL TO PUBLIC is reconstructed
>>>> without those clauses, while still producing semantically equivalent
>>>> DDL. I'm not sure whether this is intentional or if the function is
>>>> expected to reproduce all clauses explicitly, but it may be worth
>>>> discussing.
>>>> Overall, the patch worked as expected in all scenarios I tested, and I
>>>> did not find any functional issues.
>>>>
>>>
>>> Thanks for the review. The omission of FOR ALL and TO PUBLIC is
>>> intentional and matches the long-standing convention used by
>>> pg_get_indexdef, pg_get_constraintdef, pg_get_viewdef, etc.: emit a clause
>>> only when it differs from the parser's default. The result is semantically
>>> equivalent to the CREATE POLICY DDL.
>>>
>>>>
>>>>
>>>> Regards,
>>>> Solai
>>>>
>>>