Re: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed - Mailing list pgsql-hackers

From Chao Li
Subject Re: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed
Date
Msg-id CAEoWx2=vRfws6C2svUWdmNA10no+g71HABDqqKdbL43adC-VuQ@mail.gmail.com
Whole thread Raw
In response to quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed  (Chao Li <li.evan.chao@gmail.com>)
Responses Re: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed
List pgsql-hackers

On Mon, Nov 17, 2025 at 5:48 PM Chao Li <li.evan.chao@gmail.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.
Attachment

pgsql-hackers by date:

Previous
From: Nazir Bilal Yavuz
Date:
Subject: Re: PRI?64 vs Visual Studio (2022)
Next
From: "Vitaly Davydov"
Date:
Subject: RE: Newly created replication slot may be invalidated by checkpoint