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

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
>