On Wed, Oct 15, 2025 at 11:00 PM Philip Alger <paalger0@gmail.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.
