Thread

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

    Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-05-29T06:50:54Z

    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.
    
    `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@gmail.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
    >