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

On Wed, Oct 15, 2025 at 11:00 PM Philip Alger <paalger0(at)gmail(dot)com> wrote:

> Hi Akshay,
>
> When applying the patch, I got a number of errors and the tests failed. I
> think it stems from:
>
> + targetTable = relation_open(tableID, NoLock);
> + relation_close(targetTable, NoLock);
>
>
> I changed them to use "AccessShareLock" and it worked, and it's probably
> relevant to change table_close to "AccessShareLock" as well. You're just
> reading from the table.
>
> + table_close(pgPolicyRel, RowExclusiveLock);
>
>
> You might move "Datum valueDatum" to the top of the function. The
> compiler screamed at me for that, so I went ahead and made that change and
> the screaming stopped.
>
> + /* Check if the policy has a TO list */
> + Datum valueDatum = heap_getattr(tuplePolicy,
>
>
Fixed all the above review comments in the v2 patch.

>
> I also don't think you need the extra parenthesis around "USING (%s)" and
> ""WITH CHECK (%s)" in the code; it seems to print it just fine without
> them. Other people might have different opinions.
>

We need to add extra parentheses for the USING and CHECK clauses. Without
them, expressions like USING true or CHECK true will throw a syntax error
at or near "true".

>
> 2) SELECT pg_get_policy_ddl('rls_tbl_1', 'rls_p8', true); -- *pretty
>> formatted DDL*
>> pg_get_policy_ddl
>> ------------------------------------------------
>> CREATE POLICY rls_p8 ON rls_tbl_1
>> AS PERMISSIVE
>> FOR ALL
>> TO regress_rls_alice, regress_rls_dave
>> USING (true)
>> ;
>>
>>
> As for the "pretty" part. In my opinion, I don't think it's necessary, and
> putting the statement terminator (;) seems strange.
>

I think the pretty format option is a nice-to-have parameter. Users can
simply set it to false if they don’t want the DDL to be formatted.
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.

>
> However, I think you're going to get a lot of opinions on what
> well-formatted SQL looks like.
>
> --
> Best,
> Phil Alger
>