Thread

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

Akshay Joshi <akshay.joshi@enterprisedb.com> — 2025-10-22T13:21:50Z

On Wed, Oct 22, 2025 at 12:51 PM jian he <jian.universality@gmail.com>
wrote:

> On Thu, Oct 16, 2025 at 8:51 PM Akshay Joshi
> <akshay.joshi@enterprisedb.com> wrote:
> >
> > Please find attached the v3 patch, which resolves all compilation errors
> and warnings.
> >
>
> drop table if exists t, ts, ts1;
> create table t(a int);
> CREATE POLICY p0 ON t FOR ALL TO PUBLIC USING (a % 2 = 1);
> SELECT pg_get_policy_ddl('t', 'p0', false);
>
>                           pg_get_policy_ddl
> ---------------------------------------------------------------------
>   CREATE POLICY p0 ON t AS PERMISSIVE FOR ALL USING (((a % 2) = 1));
> (1 row)
>
> "TO PUBLIC" part is missing, maybe it's ok.
>

I used the logic below, which did not return PUBLIC as a role. I have added
logic to default the TO clause to PUBLIC when no specific role name is
provided
valueDatum = heap_getattr(tuplePolicy,
Anum_pg_policy_polroles,
RelationGetDescr(pgPolicyRel),
&attrIsNull);
if (!attrIsNull)
{
ArrayType *policy_roles = DatumGetArrayTypePCopy(valueDatum);
int nitems = ARR_DIMS(policy_roles)[0];
Oid *roles = (Oid *) ARR_DATA_PTR(policy_roles);


>
>
> SELECT pg_get_policy_ddl(-1, 'p0', false);
> ERROR:  could not open relation with OID 4294967295
> as I mentioned in a nearby thread [1], this should be NULL instead of
> ERROR.
> [1]
> https://postgr.es/m/CACJufxGbE4uJWu1YuqdmOx+7PMBpHvX_fbRMmHu=r4SrsuW9tg@mail.gmail.com
>
> Fixed in v4 patch.

>
> IMHO, get_formatted_string is not needed, most of the time, if pretty is
> true,
> we append "\t" and "\n", for that we can simply do
> ```
>   appendStringInfo(&buf, "CREATE POLICY %s ON %s ",
>        quote_identifier(NameStr(*policyName)),
>        generate_qualified_relation_name(policy_form->polrelid));
> if (pretty)
>     appendStringInfoString(buf, "\t\n");
> ```
>
>
The get_formatted_string function is needed. Instead of using multiple if
statements for the pretty flag, it’s better to have a generic function.
This will be useful if the pretty-format DDL implementation is accepted by
the community, as it can be reused by other pg_get_<object>_ddl() DDL
functions added in the future.

>
> in pg_get_triggerdef_worker, I found the below code pattern:
>     /*
>      * In non-pretty mode, always schema-qualify the target table name for
>      * safety.  In pretty mode, schema-qualify only if not visible.
>      */
>     appendStringInfo(&buf, " ON %s ",
>                      pretty ?
>                      generate_relation_name(trigrec->tgrelid, NIL) :
>                      generate_qualified_relation_name(trigrec->tgrelid));
>
> maybe we can apply it too while construct query string:
> "CREATE POLICY %s ON %s",
>

In my opinion, the table name should always be schema-qualified, which I
have addressed in the v4 patch. The implementation of the pretty flag here
differs from pg_get_triggerdef_worker, which is used only for the target
table name. In my patch, the pretty flag adds \t and \n to each different
clause (example AS, FOR, USING ...)