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 <[email protected]> wrote: > Rebased *v13* patch is now ready for review and commit. > > On Mon, Jun 22, 2026 at 6:49 PM Akshay Joshi < > [email protected]> 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 < >> [email protected]> wrote: >> >>> >>> >>> On Mon, Jun 8, 2026 at 6:10 PM solai v <[email protected]> wrote: >>> >>>> Hi all, >>>> >>>> >>>> On Mon, Jun 8, 2026 at 5:32 PM Japin Li <[email protected]> wrote: >>>> > >>>> > On Fri, 29 May 2026 at 12:20, Akshay Joshi < >>>> [email protected]> 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 <[email protected]> >>>> 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 >>>> >>>
v14-0001-Add-pg_get_policy_ddl-to-reconstruct-CREATE.patch
Description: Binary data
