| From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
|---|---|
| To: | Philip Alger <paalger0(at)gmail(dot)com> |
| Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement |
| Date: | 2025-10-25 05:57:54 |
| Message-ID: | CANxoLDeW8YY++ekUk5-tBxmavQn0=HRj3hWxnde=tmaopos+nA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Oct 23, 2025 at 12:19 AM Philip Alger <paalger0(at)gmail(dot)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
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2025-10-25 06:45:55 | Re: Logical Replication of sequences |
| Previous Message | Florents Tselai | 2025-10-25 04:38:58 | Re: Feature: psql - display current search_path in prompt |