MIME-Version: 1.0
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>
In-Reply-To: 
 <CANxoLDccMKZXA7qWAu6bGXRqVGu_DNPFxP4ssQ5Q4yq9Hwiq-g@mail.gmail.com>
From: Philip Alger <paalger0@gmail.com>
Date: Wed, 22 Oct 2025 13:49:17 -0500
Message-ID: 
 <CAPXBC8KGRgv30fTbxPiruPTSKgn1BEcH+8BnD=vywsSFzb8e1A@mail.gmail.com>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE
 POLICY statement
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: jian he <jian.universality@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000b3f8160641c3c67a"
Archived-At: 
 <https://www.postgresql.org/message-id/CAPXBC8KGRgv30fTbxPiruPTSKgn1BEcH%2B8BnD%3DvywsSFzb8e1A%40mail.gmail.com>
Precedence: bulk

--000000000000b3f8160641c3c67a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

>> The get_formatted_string function is needed. Instead of using multiple i=
f
> statements for the pretty flag, it=E2=80=99s better to have a generic fun=
ction.
> This will be useful if the pretty-format DDL implementation is accepted b=
y
> 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 f=
or
>>      * 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.

--=20
Best,
Phil Alger

--000000000000b3f8160641c3c67a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><div dir=3D"ltr"><br></div><div=
 class=3D"gmail_quote gmail_quote_container"><blockquote class=3D"gmail_quo=
te" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204=
);padding-left:1ex"><div dir=3D"ltr"><div class=3D"gmail_quote"><blockquote=
 class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px so=
lid rgb(204,204,204);padding-left:1ex"><br></blockquote><div>The <code>get_=
formatted_string</code> function is needed. Instead of using multiple <code=
>if</code> statements for the <code>pretty</code> flag, it=E2=80=99s better=
 to have a generic function. This will be useful if the pretty-format DDL i=
mplementation is accepted by the community, as it can be reused by other pg=
_get_&lt;object&gt;_ddl() DDL functions added in the future.=C2=A0</div><bl=
ockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-lef=
t:1px solid rgb(204,204,204);padding-left:1ex">
<br>
in pg_get_triggerdef_worker, I found the below code pattern:<br>
=C2=A0 =C2=A0 /*<br>
=C2=A0 =C2=A0 =C2=A0* In non-pretty mode, always schema-qualify the target =
table name for<br>
=C2=A0 =C2=A0 =C2=A0* safety.=C2=A0 In pretty mode, schema-qualify only if =
not visible.<br>
=C2=A0 =C2=A0 =C2=A0*/<br>
=C2=A0 =C2=A0 appendStringInfo(&amp;buf, &quot; ON %s &quot;,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0pretty ?<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0generate_relation_name(trigrec-&gt;tgrelid, NIL) :<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0generate_qualified_relation_name(trigrec-&gt;tgrelid));<br>
<br>
maybe we can apply it too while construct query string:<br>
&quot;CREATE POLICY %s ON %s&quot;,<br></blockquote><div><br>In my opinion,=
 the table name should always be schema-qualified, which=C2=A0I have addres=
sed in the v4 patch. The implementation of the <code>pretty</code> flag her=
e differs from <code>pg_get_triggerdef_worker</code>, which is used only fo=
r the target table name. In my patch, the <code>pretty</code> flag adds <co=
de>\t</code> and <code>\n</code> to each different clause (example AS, FOR,=
 USING ...)<br>=C2=A0</div></div></div>
</blockquote></div><div><br clear=3D"all"></div><div>I think that&#39;s whe=
re the confusion=C2=A0lies with=C2=A0adding `pretty` to the DDL functions. =
The `pretty` flag set to `true` on other=C2=A0functions is used to &quot;no=
t&quot; show the schema name. But in the=C2=A0case of=C2=A0this function,=
=C2=A0it 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&#39;t=
 a very clear name regardless.</div><div><br></div><span class=3D"gmail_sig=
nature_prefix">-- </span><br><div dir=3D"ltr" class=3D"gmail_signature"><di=
v dir=3D"ltr">Best,=C2=A0<div>Phil Alger</div></div></div></div>

--000000000000b3f8160641c3c67a--