| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed |
| Date: | 2025-11-20 08:19:00 |
| Message-ID: | CAEoWx2=vRfws6C2svUWdmNA10no+g71HABDqqKdbL43adC-VuQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Nov 17, 2025 at 5:48 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> Hi Hackers,
>
> == Background ==
>
> While reviewing a patch, I noticed that quoteOneName() duplicates the
> logic of quote_identifier() for adding double quotes to identifiers. The
> main difference is that quoteOneName() does *not* honor the GUC
> `quote_all_identifiers`.
>
> The typical usage pattern of quoteOneName() looks like this:
>
> ```
> // Define a local buffer with size MAX_QUOTED_NAME_LEN
> // MAX_QUOTED_NAME_LEN = MAX_NAMELEN * 2 + 3 to ensure no overflow
> char attname[MAX_QUOTED_NAME_LEN];
>
> // Add quotes and copy into the stack buffer
> quoteOneName(attname, RIAttName(pk_rel,
> riinfo->pk_attnums[riinfo->nkeys - 1]));
>
> // Copy the quoted identifier into a StringInfo
> appendStringInfoString(&querybuf, attname);
> ```
>
> This pattern is expensive because:
>
> * it allocates a larger-than-necessary buffer on the stack
> * it incurs two pallocs and two data copies
>
> Looking further, the common pattern around quote_identifier() is similar:
>
> ```
> appendStringInfoString(&relations, quote_identifier(relname));
> ```
>
> This also incurs two pallocs and two copies: quote_identifier() allocates
> a temporary buffer and copies the quoted identifier into it, and then
> appendStringInfoString() may allocate and copy again.
>
> == Idea ==
>
> I'd like to propose introducing `appendStringInfoIdentifier()`, which adds
> the quoting logic directly to StringInfo. The goals are:
>
> * eventually deprecate quoteOneName() — I'm not aware of a reason why it
> should ignore the GUC `quote_all_identifiers`
> * simplify the usage patterns for quoting identifiers
> * avoid unnecessary intermediate buffers, pallocs, and data copies
>
> == About the attached patch ==
>
> Attached v1 is not intended to be the final version — it is mainly to
> demonstrate the idea and get feedback on the design direction.
>
> * 0001 implements `appendStringInfoIdentifier()` and uses it in a few
> places
> * 0002 switches ri_triggers.c to use it, resolving a complicated usage
> pattern and showing a path toward removing quoteOneName()
>
> Comments and suggestions on the overall direction would be very welcome.
>
>
CF entry added: https://commitfest.postgresql.org/patch/6240/. And here
comes the v2 patch set.
Best regards,
Chao Li (Evan)
---------------------
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Add-appendStringInfoIdentifier-to-avoid-intermedi.patch | application/octet-stream | 11.3 KB |
| v2-0004-Use-appendStringInfoIdentifier-in-more-places.patch | application/octet-stream | 35.6 KB |
| v2-0003-Remove-quoteOneName-and-related-buffer-sizing-mac.patch | application/octet-stream | 11.5 KB |
| v2-0002-Use-appendStringInfoIdentifier-throughout-ri_trig.patch | application/octet-stream | 22.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Vitaly Davydov | 2025-11-20 08:26:05 | RE: Newly created replication slot may be invalidated by checkpoint |
| Previous Message | Nazir Bilal Yavuz | 2025-11-20 08:00:24 | Re: PRI?64 vs Visual Studio (2022) |