Fixup some appendStringInfo and appendPQExpBuffer calls

david-rowley · david-rowley · commit 110d81728a0a · 2020-10-15T20:35:17.000+13:00
A number of places were using appendStringInfo() when they could have been using appendStringInfoString() instead. While there's no functionality change there, it's just more efficient to use appendStringInfoString() when no formatting is required. Likewise for some appendStringInfoString() calls which were just appending a single char. We can just use appendStringInfoChar() for that. Additionally, many places were using appendPQExpBuffer() when they could have used appendPQExpBufferStr(). Change those too. Patch by Zhijie Hou, but further searching by me found significantly more places that deserved the same treatment. Author: Zhijie Hou, David Rowley Discussion: https://postgr.es/m/cb172cf4361e4c7ba7167429070979d4@G08CNEXMBPEKD05.g08.fujitsu.local
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
@@ -2591,8 +2591,8 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
 									 quote_identifier(relname));
 				}
 				else
-					appendStringInfo(relations, "%s",
-									 quote_identifier(relname));
+					appendStringInfoString(relations,
+										   quote_identifier(relname));
 				refname = (char *) list_nth(es->rtable_names, rti - 1);
 				if (refname == NULL)
 					refname = rte->eref->aliasname;
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c
@@ -606,7 +606,7 @@ pg_output_stream_start(LogicalDecodingContext *ctx, TestDecodingData *data, Reor
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "opening a streamed block for transaction TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "opening a streamed block for transaction");
+		appendStringInfoString(ctx->out, "opening a streamed block for transaction");
 	OutputPluginWrite(ctx, last_write);
 }
 
@@ -623,7 +623,7 @@ pg_decode_stream_stop(LogicalDecodingContext *ctx,
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "closing a streamed block for transaction TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "closing a streamed block for transaction");
+		appendStringInfoString(ctx->out, "closing a streamed block for transaction");
 	OutputPluginWrite(ctx, true);
 }
 
@@ -641,7 +641,7 @@ pg_decode_stream_abort(LogicalDecodingContext *ctx,
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "aborting streamed (sub)transaction TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "aborting streamed (sub)transaction");
+		appendStringInfoString(ctx->out, "aborting streamed (sub)transaction");
 	OutputPluginWrite(ctx, true);
 }
 
@@ -660,7 +660,7 @@ pg_decode_stream_commit(LogicalDecodingContext *ctx,
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "committing streamed transaction TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "committing streamed transaction");
+		appendStringInfoString(ctx->out, "committing streamed transaction");
 
 	if (data->include_timestamp)
 		appendStringInfo(ctx->out, " (at %s)",
@@ -693,7 +693,7 @@ pg_decode_stream_change(LogicalDecodingContext *ctx,
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "streaming change for TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "streaming change for transaction");
+		appendStringInfoString(ctx->out, "streaming change for transaction");
 	OutputPluginWrite(ctx, true);
 }
 
@@ -745,6 +745,6 @@ pg_decode_stream_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
 	if (data->include_xids)
 		appendStringInfo(ctx->out, "streaming truncate for TXN %u", txn->xid);
 	else
-		appendStringInfo(ctx->out, "streaming truncate for transaction");
+		appendStringInfoString(ctx->out, "streaming truncate for transaction");
 	OutputPluginWrite(ctx, true);
 }
diff --git a/src/backend/access/rmgrdesc/dbasedesc.c b/src/backend/access/rmgrdesc/dbasedesc.c
@@ -37,7 +37,7 @@ dbase_desc(StringInfo buf, XLogReaderState *record)
 		xl_dbase_drop_rec *xlrec = (xl_dbase_drop_rec *) rec;
 		int			i;
 
-		appendStringInfo(buf, "dir");
+		appendStringInfoString(buf, "dir");
 		for (i = 0; i < xlrec->ntablespaces; i++)
 			appendStringInfo(buf, " %u/%u",
 							 xlrec->tablespace_ids[i], xlrec->db_id);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
@@ -2768,14 +2768,14 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 						 groupInfo->groupCount);
 		/* plural/singular based on methodNames size */
 		if (list_length(methodNames) > 1)
-			appendStringInfo(es->str, "s: ");
+			appendStringInfoString(es->str, "s: ");
 		else
-			appendStringInfo(es->str, ": ");
+			appendStringInfoString(es->str, ": ");
 		foreach(methodCell, methodNames)
 		{
-			appendStringInfo(es->str, "%s", (char *) methodCell->ptr_value);
+			appendStringInfoString(es->str, (char *) methodCell->ptr_value);
 			if (foreach_current_index(methodCell) < list_length(methodNames) - 1)
-				appendStringInfo(es->str, ", ");
+				appendStringInfoString(es->str, ", ");
 		}
 
 		if (groupInfo->maxMemorySpaceUsed > 0)
@@ -2882,11 +2882,11 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate,
 		if (prefixsortGroupInfo->groupCount > 0)
 		{
 			if (es->format == EXPLAIN_FORMAT_TEXT)
-				appendStringInfo(es->str, "\n");
+				appendStringInfoChar(es->str, '\n');
 			show_incremental_sort_group_info(prefixsortGroupInfo, "Pre-sorted", true, es);
 		}
 		if (es->format == EXPLAIN_FORMAT_TEXT)
-			appendStringInfo(es->str, "\n");
+			appendStringInfoChar(es->str, '\n');
 	}
 
 	if (incrsortstate->shared_info != NULL)
@@ -2925,11 +2925,11 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate,
 			if (prefixsortGroupInfo->groupCount > 0)
 			{
 				if (es->format == EXPLAIN_FORMAT_TEXT)
-					appendStringInfo(es->str, "\n");
+					appendStringInfoChar(es->str, '\n');
 				show_incremental_sort_group_info(prefixsortGroupInfo, "Pre-sorted", true, es);
 			}
 			if (es->format == EXPLAIN_FORMAT_TEXT)
-				appendStringInfo(es->str, "\n");
+				appendStringInfoChar(es->str, '\n');
 
 			if (es->workers_state)
 				ExplainCloseWorker(n, es);
diff --git a/src/backend/replication/backup_manifest.c b/src/backend/replication/backup_manifest.c
@@ -112,7 +112,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 	initStringInfo(&buf);
 	if (manifest->first_file)
 	{
-		appendStringInfoString(&buf, "\n");
+		appendStringInfoChar(&buf, '\n');
 		manifest->first_file = false;
 	}
 	else
@@ -152,7 +152,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 	enlargeStringInfo(&buf, 128);
 	buf.len += pg_strftime(&buf.data[buf.len], 128, "%Y-%m-%d %H:%M:%S %Z",
 						   pg_gmtime(&mtime));
-	appendStringInfoString(&buf, "\"");
+	appendStringInfoChar(&buf, '"');
 
 	/* Add checksum information. */
 	if (checksum_ctx->type != CHECKSUM_TYPE_NONE)
@@ -168,7 +168,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 		enlargeStringInfo(&buf, 2 * checksumlen);
 		buf.len += hex_encode((char *) checksumbuf, checksumlen,
 							  &buf.data[buf.len]);
-		appendStringInfoString(&buf, "\"");
+		appendStringInfoChar(&buf, '"');
 	}
 
 	/* Close out the object. */
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -427,7 +427,7 @@ libpqrcv_startstreaming(WalReceiverConn *conn,
 
 		if (options->proto.logical.streaming &&
 			PQserverVersion(conn->streamConn) >= 140000)
-			appendStringInfo(&cmd, ", streaming 'on'");
+			appendStringInfoString(&cmd, ", streaming 'on'");
 
 		pubnames = options->proto.logical.publication_names;
 		pubnames_str = stringlist_to_identifierstr(conn->streamConn, pubnames);
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
@@ -774,7 +774,7 @@ copy_table(Relation rel)
 		 * For non-tables, we need to do COPY (SELECT ...), but we can't just
 		 * do SELECT * because we need to not copy generated columns.
 		 */
-		appendStringInfo(&cmd, "COPY (SELECT ");
+		appendStringInfoString(&cmd, "COPY (SELECT ");
 		for (int i = 0; i < lrel.natts; i++)
 		{
 			appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i]));
diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c
@@ -660,7 +660,7 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 			else if (v->content.anybounds.first == v->content.anybounds.last)
 			{
 				if (v->content.anybounds.first == PG_UINT32_MAX)
-					appendStringInfo(buf, "**{last}");
+					appendStringInfoString(buf, "**{last}");
 				else
 					appendStringInfo(buf, "**{%u}",
 									 v->content.anybounds.first);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
@@ -1663,7 +1663,7 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 		appendStringInfo(&querybuf, ") WHERE %s AND (",
 						 constraintDef);
 	else
-		appendStringInfo(&querybuf, ") WHERE (");
+		appendStringInfoString(&querybuf, ") WHERE (");
 
 	sep = "";
 	for (i = 0; i < riinfo->nkeys; i++)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -5250,7 +5250,7 @@ get_select_query_def(Query *query, deparse_context *context,
 			appendContextKeyword(context, " FETCH FIRST ",
 								 -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
 			get_rule_expr(query->limitCount, context, false);
-			appendStringInfo(buf, " ROWS WITH TIES");
+			appendStringInfoString(buf, " ROWS WITH TIES");
 		}
 		else
 		{
@@ -11362,7 +11362,7 @@ get_range_partbound_string(List *bound_datums)
 	memset(&context, 0, sizeof(deparse_context));
 	context.buf = buf;
 
-	appendStringInfoString(buf, "(");
+	appendStringInfoChar(buf, '(');
 	sep = "";
 	foreach(cell, bound_datums)
 	{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c

Original file line number	Diff line number	Diff line change
`@@ -774,7 +774,7 @@ copy_table(Relation rel)`
`774`	`774`	`* For non-tables, we need to do COPY (SELECT ...), but we can't just`
`775`	`775`	`* do SELECT * because we need to not copy generated columns.`
`776`	`776`	`*/`
`777`		`- appendStringInfo(&cmd, "COPY (SELECT ");`
	`777`	`+ appendStringInfoString(&cmd, "COPY (SELECT ");`
`778`	`778`	`for (int i = 0; i < lrel.natts; i++)`
`779`	`779`	`{`
`780`	`780`	`appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i]));`
Original file line number	Diff line number	Diff line change
`@@ -660,7 +660,7 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,`
`660`	`660`	`else if (v->content.anybounds.first == v->content.anybounds.last)`
`661`	`661`	`{`
`662`	`662`	`if (v->content.anybounds.first == PG_UINT32_MAX)`
`663`		`- appendStringInfo(buf, "**{last}");`
	`663`	`+ appendStringInfoString(buf, "**{last}");`
`664`	`664`	`else`
`665`	`665`	`appendStringInfo(buf, "**{%u}",`
`666`	`666`	`v->content.anybounds.first);`
Original file line number	Diff line number	Diff line change
`@@ -5250,7 +5250,7 @@ get_select_query_def(Query query, deparse_context context,`
`5250`	`5250`	`appendContextKeyword(context, " FETCH FIRST ",`
`5251`	`5251`	`-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);`
`5252`	`5252`	`get_rule_expr(query->limitCount, context, false);`
`5253`		`- appendStringInfo(buf, " ROWS WITH TIES");`
	`5253`	`+ appendStringInfoString(buf, " ROWS WITH TIES");`
`5254`	`5254`	`}`
`5255`	`5255`	`else`
`5256`	`5256`	`{`
`@@ -11362,7 +11362,7 @@ get_range_partbound_string(List *bound_datums)`
`11362`	`11362`	`memset(&context, 0, sizeof(deparse_context));`
`11363`	`11363`	`context.buf = buf;`
`11364`	`11364`
`11365`		`- appendStringInfoString(buf, "(");`
	`11365`	`+ appendStringInfoChar(buf, '(');`
`11366`	`11366`	`sep = "";`
`11367`	`11367`	`foreach(cell, bound_datums)`
`11368`	`11368`	`{`