public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Akshay Joshi <akshay.joshi@enterprisedb.com>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Philip Alger <paalger0@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Date: Wed, 22 Oct 2025 18:49:10 +0530
Message-ID: <CANxoLDeaQN5=LLnUFk8RF-fGY2EjO81oWaEdAsjCfzz4--re8w@mail.gmail.com> (raw)
In-Reply-To: <15389F54-0234-412A-ADEE-78B810A35E2B@gmail.com>
References: <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
<CAPXBC8+i=c7FCcGr6OR0y=mcx3EfdXuxyk_XYgSQ7+egGvb8vA@mail.gmail.com>
<CANxoLDdef6wW=T5czPSKPsk3xWeEHTeKxxxYMucmr-HURyoOgQ@mail.gmail.com>
<CAPXBC8J-ZBqaUh7ZcC-SBzTiOZG8P7ssUx6eLKYzVNkzKajpzQ@mail.gmail.com>
<CANxoLDcGLSHDj8Ve0qyM2UWMdgSFJA-28j7dEAtMBey8D_ktdA@mail.gmail.com>
<15389F54-0234-412A-ADEE-78B810A35E2B@gmail.com>
On Tue, Oct 21, 2025 at 2:39 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> > On Oct 16, 2025, at 20:50, Akshay Joshi <akshay.joshi@enterprisedb.com>
> wrote:
> >
> > Please find attached the v3 patch, which resolves all compilation errors
> and warnings.
> >
> > On Thu, Oct 16, 2025 at 6:06 PM Philip Alger <paalger0@gmail.com> wrote:
> > Hi Akshay,
> >
> >
> > 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.
> >
> >
> > Regarding putting the terminator at the end, I think my original comment
> got cut off by my poor editing. Yes, that's what I was referring to; no
> need to append it to a new line.
> >
> > --
> > Best, Phil Alger
> > <v3-0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch>
>
> 1 - ruleutils.c
> ```
> + if (pretty)
> + {
> + /* Indent with tabs */
> + for (int i = 0; i < noOfTabChars; i++)
> + {
> + appendStringInfoString(buf, "\t");
> + }
> ```
>
> As you are adding a single char of ‘\t’, better to use
> appendStringInfoChar() that is cheaper.
>
> 2 - ruleutils.c
> ```
> + initStringInfo(&buf);
> +
> + targetTable = relation_open(tableID, AccessShareLock);
> ```
>
> Looks like only usage of opening the table is to get the table name. In
> that case, why don’t just call get_rel_name(tableID) and store the table
> name in a local variable?
>
Fixed the above review comments in the v4 patch. I have used
generate_qualified_relation_name(tableID) instead of get_rel_name(tableID).
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
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: akshay.joshi@enterprisedb.com, li.evan.chao@gmail.com, paalger0@gmail.com
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
In-Reply-To: <CANxoLDeaQN5=LLnUFk8RF-fGY2EjO81oWaEdAsjCfzz4--re8w@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