On Mon, Nov 17, 2025 at 5:48 PM Chao Li <[email protected]> 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/
v2-0001-Add-appendStringInfoIdentifier-to-avoid-intermedi.patch
Description: Binary data
v2-0004-Use-appendStringInfoIdentifier-in-more-places.patch
Description: Binary data
v2-0003-Remove-quoteOneName-and-related-buffer-sizing-mac.patch
Description: Binary data
v2-0002-Use-appendStringInfoIdentifier-throughout-ri_trig.patch
Description: Binary data
