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

Thanks, Mark, for your review comments and the updated patch.

I’ve incorporated your changes and prepared a combined v5 patch. The v5 patch is attached for further review.

Hi everyone,

On Thu, Oct 16, 2025 at 01:47:53PM +0530, Akshay Joshi wrote:
>
>
> On Wed, Oct 15, 2025 at 10:55 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
>     Hello,
>
>     I have reviewed this patch before and provided a number of comments that
>     have been addressed by Akshay (so I encourage you to list my name and
>     this address in a Reviewed-by trailer line in the commit message).  One
>     thing I had not noticed is that while this function has a "pretty" flag,
>     it doesn't use it to pass anything to pg_get_expr_worker()'s prettyFlags
>     argument, and I think it should -- probably just
>
>       prettyFlags = GET_PRETTY_FLAGS(pretty);
>
>     same as pg_get_querydef() does.

Kinda sorta similar thought, I've noticed some existing functions like
pg_get_constraintdef make the "pretty" flag optional, so I'm wondering
if that scheme is also preferred here.

I've attached a small diff to the original
0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch to
illustrate the additional work to follow suit, if so desired.

Regards,
Mark
--
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

Hi Akshay,

As for the statement terminator, it’s useful to include it, while running multiple queries together could result in a syntax error. In my opinion, there’s no harm in providing the statement terminator.

However, I’ve modified the logic to add the statement terminator at the end instead of appending to a new line. 

Regarding putting the terminator at the end, I think my original comment got cut off by my poor editing. Yes, that's what I was referring to; no need to append it to a new line. 

--

Best, Phil Alger

Thanks for the clarification. However, I still believe this is out of scope for the CREATE POLICY DDL. The command ALTER TABLE ... ENABLE ROW LEVEL SECURITY seems more appropriate as part of the CREATE TABLE reconstruction rather than CREATE POLICY.

That said, I’m open to adding it if the majority feels it should be included in this feature.

Em sex., 7 de nov. de 2025 às 11:27, Akshay Joshi <akshay.joshi@enterprisedb.com> escreveu:

I don’t think we need that statement. Could you please elaborate on where exactly it needs to be added?

well, these pg_get_..._ddl() functions will be cool for compare/clone schemas in a multi tenant world.

Instead of dump/sed/restore a schema to create a new one, I could use something like

select pg_get_table_ddl(oid) from pg_class where nspname = 'customer_050' and relkind = 'r' union all

select pg_get_constraint_ddl(oid) from pg_constraint inner join pg_class on ... where ... union all

select pg_get_trigger_ddl(oid) from pg_trigger inner join pg_class on ... where ... union all

...

And pg_get_policy_ddl() will be part of these union all selects

Because that would be good to worry about create that only if it does not exists or drop first too.

regards

Marcos

Hi Akshay,

I tested v15 on current master (57f93af36f): builds, make check passes, and
basic reconstruction, pretty mode, NULL handling, and default-clause omission
all look good. Two things.

1) Back in the v3/v4 discussion you decided the ON <table> name should always
be schema-qualified for safety (the pg_get_triggerdef_worker thread with
jian he), and that pretty here only controls formatting, not schema (Phil's
point). That reasoning didn't reach the USING / WITH CHECK expressions,
though: object references inside them are still qualified only by the caller's
search_path, so they can lose their schema and rebind to a different object
when the DDL is replayed elsewhere:

    CREATE FUNCTION s1.f(int) RETURNS bool LANGUAGE sql AS 'SELECT $1 > 0';
    CREATE FUNCTION s2.f(int) RETURNS bool LANGUAGE sql AS 'SELECT $1 < 0';
    CREATE POLICY pf ON t2 USING (s1.f(a));

    SET search_path = public, s1;
    SELECT ddl FROM pg_get_policy_ddl('t2', 'pf') AS ddl;
    --  CREATE POLICY pf ON public.t2 USING (f(a));      -- s1. dropped

    SET search_path = public, s2;
    CREATE POLICY pf ON t2 USING (f(a));                 -- now s2.f,
opposite meaning

So within one statement the ON clause is always qualified but the expression
body isn't -- and as Phil noted, the pretty flag can't fix this, since
pg_get_expr qualifies by search_path visibility regardless of pretty.

Worth settling the contract, given where this function sits. The old
pg_get_viewdef / ruledef / indexdef functions are search_path-aware and leave
qualification to the caller (pg_dump sets search_path empty around them so the
output is portable). The new pg_get_*_ddl functions committed so far (role,
database, tablespace) are on global, schemaless objects, so the question never
came up -- pg_get_policy_ddl is the first of the family whose output embeds
schema-qualifiable references. Your own pg_get_table_ddl patch already hit
this and deparses under a controlled search_path (narrowed to pg_catalog), so
the most consistent fix is to do the same here (NewGUCNestLevel + set_config);
an empty search_path already yields the fully-qualified s1.f(a). If instead
the intent is to follow the caller's search_path, that's fine too, but it
should be documented (and SET search_path = '' noted as the way to get
portable DDL).

2) The doc calls the second parameter policy_name, but the actual argument is
policyname, so the documented named-argument call fails:

    SELECT * FROM pg_get_policy_ddl("table" => 't'::regclass,
policy_name => 'p_all');
    ERROR:  function pg_get_policy_ddl(table => regclass, policy_name
=> unknown) does not exist

Regards,
Rui

Hi Akshay,

v16 fixes both points from my last mail. Verified on master 5f14f82280:
builds, rowsecurity passes, and the s1.f/s2.f case now round-trips under a
hostile search_path. Nice that the rls_s1/rls_s2 test pins it.

I also diffed the generated DDL against pg_dump for the policies in the RLS
regression tests and pg_dump's own dump-test corpus (002_pg_dump.pl): they
match in every case except multi-role ordering (e.g. rls_p8 -- the function
keeps the declared order, TO regress_rls_dave, regress_rls_alice, while pg_dump
sorts by role OID), which doesn't matter since role order in a policy isn't
significant.

One thing left over from point 1: func-info.sgml doesn't state that object
references in the output are always schema-qualified. That's the function's
contract now, and it differs from pg_get_viewdef / ruledef (which follow the
caller's search_path), so a sentence would help. Empty search_path is also
exactly what pg_dump uses (ALWAYS_SECURE_SEARCH_PATH_SQL), so this is the
right call.

A tiny style nit, in pg_get_policy_ddl_internal():

    Assert(!attrIsNull);
    {
        ArrayType  *policy_roles = DatumGetArrayTypeP(valueDatum);

a blank line after the Assert would read a bit better.

Neither is a blocker -- looks ready to me.

Regards,
Rui

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

On Mon, Jun 8, 2026 at 6:10 PM solai v <solai.cdac@gmail.com> wrote:

Hi all,


On Mon, Jun 8, 2026 at 5:32 PM Japin Li <japinli@hotmail.com> wrote:
>
> On Fri, 29 May 2026 at 12:20, Akshay Joshi <akshay.joshi@enterprisedb.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@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
>


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

Thanks for the clarification. However, I still believe this is out of scope for the CREATE POLICY DDL. The command ALTER TABLE ... ENABLE ROW LEVEL SECURITY seems more appropriate as part of the CREATE TABLE reconstruction rather than CREATE POLICY.

That said, I’m open to adding it if the majority feels it should be included in this feature.

On Fri, Nov 7, 2025 at 8:18 PM Marcos Pegoraro <marcos@f10.com.br> wrote:

Em sex., 7 de nov. de 2025 às 11:27, Akshay Joshi <akshay.joshi@enterprisedb.com> escreveu:

I don’t think we need that statement. Could you please elaborate on where exactly it needs to be added?

well, these pg_get_..._ddl() functions will be cool for compare/clone schemas in a multi tenant world.

Instead of dump/sed/restore a schema to create a new one, I could use something like

select pg_get_table_ddl(oid) from pg_class where nspname = 'customer_050' and relkind = 'r' union all

select pg_get_constraint_ddl(oid) from pg_constraint inner join pg_class on ... where ... union all

select pg_get_trigger_ddl(oid) from pg_trigger inner join pg_class on ... where ... union all

...

And pg_get_policy_ddl() will be part of these union all selects

Because that would be good to worry about create that only if it does not exists or drop first too.

regards

Marcos

On Thu, Nov 20, 2025 at 5:27 PM Akshay Joshi
<akshay.joshi@enterprisedb.com> wrote:
>
> Attached is the v8 patch for your review, with updated variable names and a rebase applied.
>
hi.

+     <tbody>
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_get_policy_ddl</primary>
+        </indexterm>
+        <function>pg_get_policy_ddl</function>
+        ( <parameter>table</parameter> <type>regclass</type>,
<parameter>policy_name</parameter> <type>name</type>, <optional>
<parameter>pretty</parameter> <type>boolean</type> </optional> )
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Reconstructs the <command>CREATE POLICY</command> statement from the
+        system catalogs for a specified table and policy name. The result is a
+        comprehensive <command>CREATE POLICY</command> statement.
+       </para></entry>
+      </row>
+     </tbody>

 ( <parameter>table</parameter> <type>regclass</type> ...
this line is way too long, we can split it into several lines, it
won't affect the appearance.

like:
        <function>pg_get_policy_ddl</function>
        ( <parameter>table</parameter> <type>regclass</type>,
          <parameter>policy_name</parameter> <type>name</type>,
          <optional> <parameter>pretty</parameter>
<type>boolean</type> </optional> )
        <returnvalue>text</returnvalue>

Also, the explanation does not mention that the default value of
pretty is false.


index 2d946d6d9e9..a5e22374668 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -657,6 +657,12 @@ LANGUAGE INTERNAL
 STRICT VOLATILE PARALLEL UNSAFE
 AS 'pg_replication_origin_session_setup';

+CREATE OR REPLACE FUNCTION
+  pg_get_policy_ddl(tableID regclass, policyName name, pretty bool
DEFAULT false)
+RETURNS text
+LANGUAGE INTERNAL
+AS 'pg_get_policy_ddl';
+

The partial upper casing above has no effect; it's the same as
``pg_get_policy_ddl(tableid regclass, policyname name, pretty bool
DEFAULT false)``

--
jian
https://www.enterprisedb.com/

Hi, Akshay

On Fri, 22 May 2026 at 19:02, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
> Hi hackers,                                                                                                             
>                     
>
> Following the recently committed pg_get_database_ddl(), which adopted a VARIADIC options text[] style for
> DDL-reconstruction functions, here is a patch in the same spirit for row-level security policies.
>
> The new function:
>     pg_get_policy_ddl(table regclass, policy_name name, VARIADIC options text[]) RETURNS setof text
>
> Reconstructs the CREATE POLICY statement for the named policy on the given table, returning the result as a single row.
>
> The currently supported option is pretty (boolean) for formatted output.                                                 
>       
>     SELECT * FROM pg_get_policy_ddl('rls_table', 'pol1');
>     SELECT * FROM pg_get_policy_ddl('rls_table', 'pol1', 'pretty', 'true');
>
> NULL inputs for table or policy_name return no rows. Unknown option names, invalid boolean values, and duplicate options
> are reported as errors consistent with the pattern established by pg_get_database_ddl().
>
> The patch includes documentation updates in func-info.sgml and regression tests in rowsecurity.sql covering
> PERMISSIVE/RESTRICTIVE, each command type (ALL/SELECT/INSERT/UPDATE/DELETE), TO role lists, both USING and WITH CHECK
> clauses, pretty/non-pretty output, and the error paths above.
>
> Patch is ready for review.
>
> On Mon, Jan 5, 2026 at 8:00 PM jian he <jian.universality@gmail.com> wrote:
>
>  On Thu, Nov 20, 2025 at 5:27 PM Akshay Joshi
>  <akshay.joshi@enterprisedb.com> wrote:
>  >
>  > Attached is the v8 patch for your review, with updated variable names and a rebase applied.
>  >
>  hi.
>
>  +     <tbody>
>  +      <row>
>  +       <entry role="func_table_entry"><para role="func_signature">
>  +        <indexterm>
>  +         <primary>pg_get_policy_ddl</primary>
>  +        </indexterm>
>  +        <function>pg_get_policy_ddl</function>
>  +        ( <parameter>table</parameter> <type>regclass</type>,
>  <parameter>policy_name</parameter> <type>name</type>, <optional>
>  <parameter>pretty</parameter> <type>boolean</type> </optional> )
>  +        <returnvalue>text</returnvalue>
>  +       </para>
>  +       <para>
>  +        Reconstructs the <command>CREATE POLICY</command> statement from the
>  +        system catalogs for a specified table and policy name. The result is a
>  +        comprehensive <command>CREATE POLICY</command> statement.
>  +       </para></entry>
>  +      </row>
>  +     </tbody>
>
>   ( <parameter>table</parameter> <type>regclass</type> ...
>  this line is way too long, we can split it into several lines, it
>  won't affect the appearance.
>
>  like:
>          <function>pg_get_policy_ddl</function>
>          ( <parameter>table</parameter> <type>regclass</type>,
>            <parameter>policy_name</parameter> <type>name</type>,
>            <optional> <parameter>pretty</parameter>
>  <type>boolean</type> </optional> )
>          <returnvalue>text</returnvalue>
>
>  Also, the explanation does not mention that the default value of
>  pretty is false.
>
>  index 2d946d6d9e9..a5e22374668 100644
>  --- a/src/backend/catalog/system_functions.sql
>  +++ b/src/backend/catalog/system_functions.sql
>  @@ -657,6 +657,12 @@ LANGUAGE INTERNAL
>   STRICT VOLATILE PARALLEL UNSAFE
>   AS 'pg_replication_origin_session_setup';
>
>  +CREATE OR REPLACE FUNCTION
>  +  pg_get_policy_ddl(tableID regclass, policyName name, pretty bool
>  DEFAULT false)
>  +RETURNS text
>  +LANGUAGE INTERNAL
>  +AS 'pg_get_policy_ddl';
>  +
>
>  The partial upper casing above has no effect; it's the same as
>  ``pg_get_policy_ddl(tableid regclass, policyname name, pretty bool
>  DEFAULT false)``
>

Thanks for updating the patch.  Just one nitpick below.

+               append_ddl_option(&buf, pretty, 4, "USING (%s)",
+                                                 TextDatumGetCString(expr));

The expression string already contains the parentheses, so we can omit them
here, as well as in the WITH CHECK clause.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hi all,

On Wed, Jul 1, 2026 at 4:01 PM Akshay Joshi
<akshay.joshi@enterprisedb.com> wrote:
>
>
>
> On Wed, Jul 1, 2026 at 2:44 PM solai v <solai.cdac@gmail.com> wrote:
>>
>> Hi all,
>>
>>
>> On Tue, Jun 30, 2026 at 4:48 PM Akshay Joshi
>> <akshay.joshi@enterprisedb.com> wrote:
>> >
>> > 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.
>> >
>>
>>
>> I reviewed and tested the v14 patch on the latest master. The patch
>> applied cleanly, built successfully, and passed make check without any
>> regression failures. I verified the new function signature and
>> confirmed that pg_get_policy_ddl() now uses the new interface with the
>> boolean DEFAULT false argument. Also I tested the function with a
>> variety of row-level security policies, including: Basic policy
>> reconstruction, PERMISSIVE and RESTRICTIVE policies, Different command
>> types (SELECT, INSERT, UPDATE, DELETE), Policies with multiple roles
>> and the PUBLIC role, Quoted table, policy, and role names, USING and
>> WITH CHECK clauses, Complex expressions including EXISTS, nested
>> subqueries, CASE expressions, COALESCE, ANY/ALL operators, and boolean
>> expressions, NULL inputs, invalid relation names, and non-existent
>> policies. The generated DDL was correct as per the intent, and the
>> expected errors were verified for the invalid inputs. I also verified
>> that the generated DDL is executable by dropping an existing policy,
>> recreating it using the output of pg_get_policy_ddl(), and confirming
>> that the recreated policy was reconstructed successfully again by the
>> function.
>> One observation I noted is that for policies using default attributes
>> (TO PUBLIC and AS PERMISSIVE), the generated DDL omits those clauses
>> and produces a semantically equivalent statement which seems
>> intentional but thought of sending it here for further clarification.
>> Overall, I did not encounter any functional issues during testing. The
>> patch looks good to me and worked as expected in all the scenarios I
>> tested.
>
>
>     Yes, it's intentional for all pg_get_***_ddl functions. Clause with defaults won't be reconstructed.
>>
>>

Ok. Thank you for the clarification.

Regards,
Solai

Make the pretty flag a default parameter by adding CREATE OR REPLACE FUNCTION in system_functions.sql.
Attached is the v7 patch, which is ready for review.

Hi Hackers,

Added a new #define GET_DDL_PRETTY_FLAGS because the existing #define GET_PRETTY_FLAGS is not suitable for formatting reconstructed DDLs. The existing #define GET_PRETTY_FLAGS always indents the code, regardless of whether the flag is set to true or false, which is not the desired behavior for pg_get_<object>_ddl functions.

Updated the logic of the get_formatted_string function based on Tim Waizenegger’s suggestion.

I am attaching the new v6 patch, which is ready for review.

On Tue, Oct 28, 2025 at 3:08 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, Mark, for your review comments and the updated patch.

I’ve incorporated your changes and prepared a combined v5 patch. The v5 patch is attached for further review.

On Mon, Oct 27, 2025 at 10:15 PM Mark Wong <markwkm@gmail.com> wrote:

Hi everyone,

On Thu, Oct 16, 2025 at 01:47:53PM +0530, Akshay Joshi wrote:
>
>
> On Wed, Oct 15, 2025 at 10:55 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
>     Hello,
>
>     I have reviewed this patch before and provided a number of comments that
>     have been addressed by Akshay (so I encourage you to list my name and
>     this address in a Reviewed-by trailer line in the commit message).  One
>     thing I had not noticed is that while this function has a "pretty" flag,
>     it doesn't use it to pass anything to pg_get_expr_worker()'s prettyFlags
>     argument, and I think it should -- probably just
>
>       prettyFlags = GET_PRETTY_FLAGS(pretty);
>
>     same as pg_get_querydef() does.

Kinda sorta similar thought, I've noticed some existing functions like
pg_get_constraintdef make the "pretty" flag optional, so I'm wondering
if that scheme is also preferred here.

I've attached a small diff to the original
0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch to
illustrate the additional work to follow suit, if so desired.

Regards,
Mark
--
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

Thanks, Mark, for your review comments and the updated patch.

I’ve incorporated your changes and prepared a combined v5 patch. The v5 patch is attached for further review.

On Mon, Oct 27, 2025 at 10:15 PM Mark Wong <markwkm@gmail.com> wrote:

Hi everyone,

On Thu, Oct 16, 2025 at 01:47:53PM +0530, Akshay Joshi wrote:
>
>
> On Wed, Oct 15, 2025 at 10:55 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
>     Hello,
>
>     I have reviewed this patch before and provided a number of comments that
>     have been addressed by Akshay (so I encourage you to list my name and
>     this address in a Reviewed-by trailer line in the commit message).  One
>     thing I had not noticed is that while this function has a "pretty" flag,
>     it doesn't use it to pass anything to pg_get_expr_worker()'s prettyFlags
>     argument, and I think it should -- probably just
>
>       prettyFlags = GET_PRETTY_FLAGS(pretty);
>
>     same as pg_get_querydef() does.

Kinda sorta similar thought, I've noticed some existing functions like
pg_get_constraintdef make the "pretty" flag optional, so I'm wondering
if that scheme is also preferred here.

I've attached a small diff to the original
0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch to
illustrate the additional work to follow suit, if so desired.

Regards,
Mark
--
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

+ targetTable = relation_open(tableID, NoLock);

+ relation_close(targetTable, NoLock);

+ table_close(pgPolicyRel, RowExclusiveLock);