public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Mark Wong <markwkm@gmail.com>
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: Álvaro Herrera <alvherre@kurilemu.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Date: Mon, 27 Oct 2025 09:45:10 -0700
Message-ID: <aP-hllbRdgqbmB8L@ltdrgnflg2> (raw)
In-Reply-To: <CANxoLDfXKWRQ0KZFtaChr2NX3UWSQq3ji7OJqBp-_-th2Zj6Fg@mail.gmail.com>
References: <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
<202510151529.s3fpwsgben57@alvherre.pgsql>
<CANxoLDfXKWRQ0KZFtaChr2NX3UWSQq3ji7OJqBp-_-th2Zj6Fg@mail.gmail.com>
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
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',
Attachments:
[text/plain] for-optional-pretty.diff (4.6K, 2-for-optional-pretty.diff)
download | inline 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',
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: markwkm@gmail.com, akshay.joshi@enterprisedb.com, alvherre@kurilemu.de
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
In-Reply-To: <aP-hllbRdgqbmB8L@ltdrgnflg2>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox