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

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,


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.

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.
However, I think you're going to get a lot of opinions on what
well-formatted SQL looks like.

-- 
Best,
Phil Alger