Date: Mon, 27 Oct 2025 09:45:10 -0700
From: Mark Wong <markwkm@gmail.com>
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: =?iso-8859-1?Q?=C1lvaro?= Herrera <alvherre@kurilemu.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE
 POLICY statement
Message-ID: <aP-hllbRdgqbmB8L@ltdrgnflg2>
References: 
 <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
 <202510151529.s3fpwsgben57@alvherre.pgsql>
 <CANxoLDfXKWRQ0KZFtaChr2NX3UWSQq3ji7OJqBp-_-th2Zj6Fg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="9ruR+s6V65BBysym"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CANxoLDfXKWRQ0KZFtaChr2NX3UWSQq3ji7OJqBp-_-th2Zj6Fg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aP-hllbRdgqbmB8L%40ltdrgnflg2>
Precedence: bulk


--9ruR+s6V65BBysym
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

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

--9ruR+s6V65BBysym
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=for-optional-pretty.diff

diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index 4b9c661c20b..72b836cd082 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3827,19 +3827,29 @@ acl      | {postgres=arwdDxtm/postgres,foo=r/postgres}
          <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>, <parameter>pretty</parameter> <type>boolean</type> )
+        ( <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 CREATE POLICY statement from the system catalogs for a specified table and policy name.
-        When the pretty flag is set to true, the function returns a well-formatted DDL statement.
-        The result is a comprehensive <command>CREATE POLICY</command> statement.
+        Reconstructs the <command>CREATE POLICY statement</command> 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>
     </tgroup>
    </table>
 
+  <para>
+   Most of the functions that reconstruct (decompile) database objects have an
+   optional <parameter>pretty</parameter> flag, which if
+   <literal>true</literal> causes the result to be
+   <quote>pretty-printed</quote>.  Pretty-printing adds whitespace for
+   legibility. Passing <literal>false</literal> for the
+   <parameter>pretty</parameter> parameter yields the same result as omitting
+   the parameter.
+  </para>
+
   </sect2>
 
   </sect1>
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index c05e4786703..e6d21a1d00e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -552,6 +552,7 @@ static void get_formatted_string(StringInfo buf,
 								 bool pretty,
 								 int noOfTabChars,
 								 const char *fmt,...) pg_attribute_printf(4, 5);
+static char    *pg_get_policy_ddl_worker(Oid tableID, Name policyName, bool pretty);
 
 #define only_marker(rte)  ((rte)->inh ? "" : "ONLY ")
 
@@ -13788,12 +13789,41 @@ get_formatted_string(StringInfo buf, bool pretty, int noOfTabChars, const char *
  * policyName - Name of the policy for which to generate the DDL.
  * pretty - If true, format the DDL with indentation and line breaks.
  */
+
 Datum
 pg_get_policy_ddl(PG_FUNCTION_ARGS)
+{
+	Oid			tableID = PG_GETARG_OID(0);
+	Name		policyName = PG_GETARG_NAME(1);
+	char	   *res;
+
+	res = pg_get_policy_ddl_worker(tableID, policyName, false);
+
+	if (res == NULL)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(string_to_text(res));
+}
+
+Datum
+pg_get_policy_ddl_ext(PG_FUNCTION_ARGS)
 {
 	Oid			tableID = PG_GETARG_OID(0);
 	Name		policyName = PG_GETARG_NAME(1);
 	bool		pretty = PG_GETARG_BOOL(2);
+	char	   *res;
+
+	res = pg_get_policy_ddl_worker(tableID, policyName, pretty);
+
+	if (res == NULL)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(string_to_text(res));
+}
+
+static char *
+pg_get_policy_ddl_worker(Oid tableID, Name policyName, bool pretty)
+{
 	bool		attrIsNull;
 	int			prettyFlags;
 	Datum		valueDatum;
@@ -13807,7 +13837,7 @@ pg_get_policy_ddl(PG_FUNCTION_ARGS)
 
 	/* Validate that the relation exists */
 	if (!OidIsValid(tableID) || get_rel_name(tableID) == NULL)
-		PG_RETURN_NULL();
+		return NULL;
 
 	initStringInfo(&buf);
 
@@ -13935,5 +13965,5 @@ pg_get_policy_ddl(PG_FUNCTION_ARGS)
 	systable_endscan(sscan);
 	table_close(pgPolicyRel, AccessShareLock);
 
-	PG_RETURN_TEXT_P(string_to_text(buf.data));
+	return buf.data;
 }
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 536c5a857da..3bfaf34d535 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4023,7 +4023,10 @@
   prosrc => 'pg_get_function_sqlbody' },
 { oid => '8811', descr => 'get CREATE statement for policy',
   proname => 'pg_get_policy_ddl', prorettype => 'text',
-  proargtypes => 'regclass name bool', prosrc => 'pg_get_policy_ddl' },
+  proargtypes => 'regclass name', prosrc => 'pg_get_policy_ddl' },
+{ oid => '8812', descr => 'get CREATE statement for policy with pretty-print option',
+  proname => 'pg_get_policy_ddl', prorettype => 'text',
+  proargtypes => 'regclass name bool', prosrc => 'pg_get_policy_ddl_ext' },
 
 { oid => '1686', descr => 'list of SQL keywords',
   proname => 'pg_get_keywords', procost => '10', prorows => '500',

--9ruR+s6V65BBysym--