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

*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
>>
>