| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed |
| Date: | 2025-11-17 09:48:43 |
| Message-ID: | CAEoWx2=g2RVkxXB=JzWphgfg4QGV+spaA3PQ1rBM2iMehrVvjg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Add-appendStringInfoIdentifier-and-use-it-in-a-fe.patch | application/octet-stream | 10.5 KB |
| v1-0002-Apply-appendStringInfoIdentifier-in-ri_triggers.c.patch | application/octet-stream | 21.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | v@viktorh.net | 2025-11-17 10:00:31 | Re: ON CONFLICT DO SELECT (take 3) |
| Previous Message | Michael Banck | 2025-11-17 09:39:56 | Re: GNU/Hurd portability patches |