public inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Philip Alger <paalger0@gmail.com>
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Date: Wed, 15 Oct 2025 12:30:35 -0500
Message-ID: <CAPXBC8+i=c7FCcGr6OR0y=mcx3EfdXuxyk_XYgSQ7+egGvb8vA@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
References: <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>

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


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: paalger0@gmail.com, akshay.joshi@enterprisedb.com
  Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
  In-Reply-To: <CAPXBC8+i=c7FCcGr6OR0y=mcx3EfdXuxyk_XYgSQ7+egGvb8vA@mail.gmail.com>

* 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