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 <lic@highgo.com>
Discussion: https://postgr.es/m/CAEoWx2=g2RVkxXB=JzWphgfg4QGV+spaA3PQ1rBM2iMehrVvjg@mail.gmail.com

Author: Chao Li (Evan)

src/backend/catalog/objectaddress.c

@@ -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;

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);
 			}

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);
 }

src/backend/utils/adt/ruleutils.c

@@ -13052,33 +13052,25 @@ 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 <ctype.h> macros here, but they might yield unwanted
 	 * locale-specific results...
 	 */
 	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;
 }
 

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);