public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Akshay Joshi <akshay.joshi@enterprisedb.com>
To: Philip Alger <paalger0@gmail.com>
Cc: jian he <jian.universality@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Date: Sat, 25 Oct 2025 11:27:54 +0530
Message-ID: <CANxoLDeW8YY++ekUk5-tBxmavQn0=HRj3hWxnde=tmaopos+nA@mail.gmail.com> (raw)
In-Reply-To: <CAPXBC8KGRgv30fTbxPiruPTSKgn1BEcH+8BnD=vywsSFzb8e1A@mail.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>
<CACJufxGvfatGyM7RFqMcpSYyNkm-hUWYw2WRKn_=sQwVPGs+CA@mail.gmail.com>
<CANxoLDccMKZXA7qWAu6bGXRqVGu_DNPFxP4ssQ5Q4yq9Hwiq-g@mail.gmail.com>
<CAPXBC8KGRgv30fTbxPiruPTSKgn1BEcH+8BnD=vywsSFzb8e1A@mail.gmail.com>
On Thu, Oct 23, 2025 at 12:19 AM Philip Alger <paalger0@gmail.com> wrote:
>
>
>
>>> The get_formatted_string function is needed. Instead of using multiple
>> if statements for the pretty flag, it’s better to have a generic
>> function. This will be useful if the pretty-format DDL implementation is
>> accepted by the community, as it can be reused by other
>> pg_get_<object>_ddl() DDL functions added in the future.
>>
>>>
>>> in pg_get_triggerdef_worker, I found the below code pattern:
>>> /*
>>> * In non-pretty mode, always schema-qualify the target table name
>>> for
>>> * safety. In pretty mode, schema-qualify only if not visible.
>>> */
>>> appendStringInfo(&buf, " ON %s ",
>>> pretty ?
>>> generate_relation_name(trigrec->tgrelid, NIL) :
>>> generate_qualified_relation_name(trigrec->tgrelid));
>>>
>>> maybe we can apply it too while construct query string:
>>> "CREATE POLICY %s ON %s",
>>>
>>
>> In my opinion, the table name should always be schema-qualified, which I
>> have addressed in the v4 patch. The implementation of the pretty flag
>> here differs from pg_get_triggerdef_worker, which is used only for the
>> target table name. In my patch, the pretty flag adds \t and \n to each
>> different clause (example AS, FOR, USING ...)
>>
>>
>
> I think that's where the confusion lies with adding `pretty` to the DDL
> functions. The `pretty` flag set to `true` on other functions is used to
> "not" show the schema name. But in the case of this function, it is used to
> format the statement. I wonder if the word `format` or `pretty_format` is a
> better name? `pretty` itself in the codebase isn't a very clear name
> regardless.
>
In my opinion, we should first decide whether we want the DDL statement to
be in a 'pretty' format or not. Personally, I believe it should be, since
parsing a one-line DDL statement can be quite complex and difficult for
users of this function. From a user’s perspective, having the entire DDL in
a single line makes it harder to read and understand. The flag allows users
to disable the pretty format by passing false.
If the community agrees on this approach, we can then think about choosing
a more appropriate word for it.
>
> --
> 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: akshay.joshi@enterprisedb.com, paalger0@gmail.com, jian.universality@gmail.com
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
In-Reply-To: <CANxoLDeW8YY++ekUk5-tBxmavQn0=HRj3hWxnde=tmaopos+nA@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