From 70971d0e75ab61436d0328a88e5441cf2751a9b6 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Fri, 14 Nov 2025 21:44:29 +0800 Subject: [PATCH v2 1/4] Add appendStringInfoIdentifier() to avoid intermediate quoting buffers Introduce appendStringInfoIdentifier() and appendStringInfoQualifiedIdentifier(), helper functions that append an SQL identifier directly to a StringInfo while applying quoting rules when necessary. This avoids allocating and copying through temporary palloc buffers, as currently happens with quote_identifier() when used together with appendStringInfoString(). The new functions improve both readability and efficiency of call sites that construct SQL fragments, especially those that need to build qualified names such as schema.table. Convert several existing callers in objectaddress.c, explain.c and ruleutils.c to use appendStringInfoIdentifier() / appendStringInfoQualifiedIdentifier() as examples. No functional behavior change is intended. Author: Chao Li Discussion: https://postgr.es/m/CAEoWx2=g2RVkxXB=JzWphgfg4QGV+spaA3PQ1rBM2iMehrVvjg@mail.gmail.com --- src/backend/catalog/objectaddress.c | 20 ++-- src/backend/commands/explain.c | 3 +- src/backend/utils/adt/ri_triggers.c | 8 +- src/backend/utils/adt/ruleutils.c | 142 +++++++++++++++++++++++----- src/include/utils/builtins.h | 6 ++ 5 files changed, 136 insertions(+), 43 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index c75b7131ed7..fbb0b4f30f3 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -3611,9 +3611,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) else nspname = get_namespace_name(cfgForm->cfgnamespace); - appendStringInfo(&buffer, _("text search configuration %s"), - quote_qualified_identifier(nspname, - NameStr(cfgForm->cfgname))); + appendStringInfoQualifiedIdentifier(&buffer, + _("text search configuration "), + nspname, NameStr(cfgForm->cfgname), NULL); ReleaseSysCache(tup); break; } @@ -4883,8 +4883,7 @@ getObjectIdentityParts(const ObjectAddress *object, if (attr) { - appendStringInfo(&buffer, ".%s", - quote_identifier(attr)); + appendStringInfoIdentifier(&buffer, ".", attr, NULL); if (objname) *objname = lappend(*objname, attr); } @@ -5395,8 +5394,7 @@ getObjectIdentityParts(const ObjectAddress *object, object->objectId); break; } - appendStringInfoString(&buffer, - quote_identifier(nspname)); + appendStringInfoIdentifier(&buffer, NULL, nspname, NULL); if (objname) *objname = list_make1(nspname); break; @@ -5739,16 +5737,12 @@ getObjectIdentityParts(const ObjectAddress *object, defacl = (Form_pg_default_acl) GETSTRUCT(tup); username = GetUserNameFromId(defacl->defaclrole, false); - appendStringInfo(&buffer, - "for role %s", - quote_identifier(username)); + appendStringInfoIdentifier(&buffer, "for role ", username, NULL); if (OidIsValid(defacl->defaclnamespace)) { schema = get_namespace_name_or_temp(defacl->defaclnamespace); - appendStringInfo(&buffer, - " in schema %s", - quote_identifier(schema)); + appendStringInfoIdentifier(&buffer, " in schema ", schema, NULL); } else schema = NULL; diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 7e699f8595e..cc979737845 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -1705,8 +1705,7 @@ ExplainNode(PlanState *planstate, List *ancestors, explain_get_index_name(bitmapindexscan->indexid); if (es->format == EXPLAIN_FORMAT_TEXT) - appendStringInfo(es->str, " on %s", - quote_identifier(indexname)); + appendStringInfoIdentifier(es->str, " on ", indexname, NULL); else ExplainPropertyText("Index Name", indexname, es); } diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 059fc5ebf60..9e13f526994 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -395,7 +395,6 @@ RI_FKey_check(TriggerData *trigdata) { quoteOneName(attname, RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1])); - appendStringInfo(&querybuf, "SELECT 1 FROM (SELECT %s AS r FROM %s%s x", attname, pk_only, pkrelname); @@ -2095,7 +2094,6 @@ ri_GenerateQualCollation(StringInfo buf, Oid collation) HeapTuple tp; Form_pg_collation colltup; char *collname; - char onename[MAX_QUOTED_NAME_LEN]; /* Nothing to do if it's a noncollatable data type */ if (!OidIsValid(collation)) @@ -2111,10 +2109,8 @@ ri_GenerateQualCollation(StringInfo buf, Oid collation) * We qualify the name always, for simplicity and to ensure the query is * not search-path-dependent. */ - quoteOneName(onename, get_namespace_name(colltup->collnamespace)); - appendStringInfo(buf, " COLLATE %s", onename); - quoteOneName(onename, collname); - appendStringInfo(buf, ".%s", onename); + appendStringInfoIdentifier(buf, " COLLATE ", get_namespace_name(colltup->collnamespace), NULL); + appendStringInfoIdentifier(buf, ".", collname, NULL); ReleaseSysCache(tp); } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 556ab057e5a..1d767deb6c4 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -13052,25 +13052,17 @@ printSubscripts(SubscriptingRef *sbsref, deparse_context *context) } } -/* - * quote_identifier - Quote an identifier only if needed - * - * When quotes are needed, we palloc the required space; slightly - * space-wasteful but well worth it for notational simplicity. - */ -const char * -quote_identifier(const char *ident) +static inline bool +is_identifier_safe(const char *ident, int *nquotes) { /* * Can avoid quoting if ident starts with a lowercase letter or underscore * and contains only lowercase letters, digits, and underscores, *and* is * not any SQL keyword. Otherwise, supply quotes. */ - int nquotes = 0; bool safe; - const char *ptr; - char *result; - char *optr; + + *nquotes = 0; /* * would like to use macros here, but they might yield unwanted @@ -13078,7 +13070,7 @@ quote_identifier(const char *ident) */ safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_'); - for (ptr = ident; *ptr; ptr++) + for (const char *ptr = ident; *ptr; ptr++) { char ch = *ptr; @@ -13092,7 +13084,7 @@ quote_identifier(const char *ident) { safe = false; if (ch == '"') - nquotes++; + (*nquotes)++; } } @@ -13115,14 +13107,20 @@ quote_identifier(const char *ident) safe = false; } - if (safe) - return ident; /* no change needed */ + return safe; +} - result = (char *) palloc(strlen(ident) + nquotes + 2 + 1); +static inline const char * +quote_identifier_internal(const char *ident, int nquotes, char **result) +{ + char *optr; - optr = result; + if (*result == NULL) + *result = (char *) palloc(strlen(ident) + nquotes + 2 + 1); + + optr = *result; *optr++ = '"'; - for (ptr = ident; *ptr; ptr++) + for (const char *ptr = ident; *ptr; ptr++) { char ch = *ptr; @@ -13133,7 +13131,107 @@ quote_identifier(const char *ident) *optr++ = '"'; *optr = '\0'; - return result; + return *result; +} + +/* + * quote_identifier - Quote an identifier only if needed + * + * When quotes are needed, we palloc the required space; slightly + * space-wasteful but well worth it for notational simplicity. + */ +const char * +quote_identifier(const char *ident) +{ + int nquotes; + bool safe; + char *result = NULL; + + safe = is_identifier_safe(ident, &nquotes); + if (safe) + return ident; /* no change needed */ + + return quote_identifier_internal(ident, nquotes, &result); +} + +/* + * appendStringInfoIdentifier + * Append an SQL identifier to a StringInfo, quoting if required. + * + * This behaves like writing prefix + quote_identifier(ident) + suffix into + * the StringInfo, but emits the identifier directly into the buffer to avoid + * an intermediate palloc. prefix and suffix may be NULL. + * + * The identifier is copied verbatim if it is safe to leave unquoted; otherwise + * it is written with double quotes and embedded double quotes are doubled. + * Quoting rules are local to ruleutils, so this helper is defined here rather + * than in stringinfo.c. + */ + +void +appendStringInfoIdentifier(StringInfo str, const char *prefix, + const char *ident, const char *suffix) +{ + int nquotes; + bool safe; + int ident_len; + int prefix_len = 0; + int suffix_len = 0; + + safe = is_identifier_safe(ident, &nquotes); + if (safe) + ident_len = strlen(ident); + else + ident_len = strlen(ident) + nquotes + 2; /* quotes + possible + * escapes */ + + if (prefix != NULL) + prefix_len = strlen(prefix); + + if (suffix != NULL) + suffix_len = strlen(suffix); + + enlargeStringInfo(str, ident_len + prefix_len + suffix_len + 1); /* +1 for trailing null */ + + if (prefix != NULL) + appendBinaryStringInfo(str, prefix, prefix_len); + + if (safe) + appendBinaryStringInfo(str, ident, ident_len); + else + { + char *result = str->data + str->len; + + (void) quote_identifier_internal(ident, nquotes, &result); + str->len += ident_len; + str->data[str->len] = '\0'; + } + + if (suffix != NULL) + appendBinaryStringInfo(str, suffix, suffix_len); +} + +/* + * appendStringInfoQualifiedIdentifier + * Append a (possibly) qualified SQL identifier to a StringInfo. + * + * Writes prefix + qualifier + '.' + ident + suffix, quoting each identifier + * component if needed. If no qualifier is given, only ident (plus optional + * prefix/suffix) is appended. prefix and suffix may be NULL. + * + * This is a convenience wrapper around appendStringInfoIdentifier(). + */ +void +appendStringInfoQualifiedIdentifier(StringInfo str, const char *prefix, + const char *qualifier, const char *ident, + const char *suffix) +{ + if (qualifier) + { + appendStringInfoIdentifier(str, prefix, qualifier, "."); + prefix = NULL; + } + appendStringInfoIdentifier(str, prefix, ident, suffix); } /* @@ -13150,8 +13248,8 @@ quote_qualified_identifier(const char *qualifier, initStringInfo(&buf); if (qualifier) - appendStringInfo(&buf, "%s.", quote_identifier(qualifier)); - appendStringInfoString(&buf, quote_identifier(ident)); + appendStringInfoIdentifier(&buf, NULL, qualifier, "."); + appendStringInfoIdentifier(&buf, NULL, ident, NULL); return buf.data; } diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h index ce6285a2c03..ff4ba7265c2 100644 --- a/src/include/utils/builtins.h +++ b/src/include/utils/builtins.h @@ -84,6 +84,12 @@ extern void generate_operator_clause(StringInfo buf, const char *leftop, Oid leftoptype, Oid opoid, const char *rightop, Oid rightoptype); +extern void appendStringInfoIdentifier(StringInfo str, const char *prefix, const char *ident, const char *suffix); +extern void appendStringInfoQualifiedIdentifier(StringInfo str, + const char *prefix, + const char *qualifier, + const char *ident, + const char *suffix); /* varchar.c */ extern int bpchartruelen(char *s, int len); -- 2.39.5 (Apple Git-154)