Cleaning up and speeding up string functions

From 6640a59cbb337dc2d7348c507858eea3efbda54f Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Sun, 28 Apr 2019 12:33:29 +1200
Subject: [PATCH 1/4] Use appendPQExpBufferStr instead of appendPQExpBuffer
 where possible

If the call does not require any printf type formatting then there is no
point in using appendPQExpBuffer, doing so just adds overhead and leaves
bad examples in the code.
---
 src/bin/pg_basebackup/streamutil.c   |   8 +-
 src/bin/pg_ctl/pg_ctl.c              |   4 +-
 src/bin/pg_dump/dumputils.c          |   4 +-
 src/bin/pg_dump/pg_backup_archiver.c |   6 +-
 src/bin/pg_dump/pg_backup_db.c       |   2 +-
 src/bin/pg_dump/pg_dump.c            | 158 +++++++++++++--------------
 src/bin/pg_dump/pg_dumpall.c         |   4 +-
 src/bin/pg_upgrade/dump.c            |   2 +-
 src/bin/psql/command.c               |   4 +-
 src/bin/psql/describe.c              |  94 ++++++++--------
 src/bin/scripts/clusterdb.c          |   2 +-
 src/bin/scripts/reindexdb.c          |   8 +-
 src/bin/scripts/vacuumdb.c           |  44 ++++----
 src/fe_utils/string_utils.c          |   4 +-
 src/interfaces/libpq/fe-auth-scram.c |  14 +--
 src/interfaces/libpq/fe-connect.c    |   4 +-
 src/interfaces/libpq/fe-protocol3.c  |   4 +-
 17 files changed, 183 insertions(+), 183 deletions(-)

diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 79f17e4089..8d8ac11b66 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -500,19 +500,19 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	/* Build query */
 	appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
 	if (is_temporary)
-		appendPQExpBuffer(query, " TEMPORARY");
+		appendPQExpBufferStr(query, " TEMPORARY");
 	if (is_physical)
 	{
-		appendPQExpBuffer(query, " PHYSICAL");
+		appendPQExpBufferStr(query, " PHYSICAL");
 		if (reserve_wal)
-			appendPQExpBuffer(query, " RESERVE_WAL");
+			appendPQExpBufferStr(query, " RESERVE_WAL");
 	}
 	else
 	{
 		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
 		if (PQserverVersion(conn) >= 100000)
 			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
-			appendPQExpBuffer(query, " NOEXPORT_SNAPSHOT");
+			appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");
 	}
 
 	res = PQexec(conn, query->data);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index dfb6c19f5a..a10bc8d545 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1481,14 +1481,14 @@ pgwin32_CommandLine(bool registration)
 		appendPQExpBuffer(cmdLine, " -e \"%s\"", event_source);
 
 	if (registration && do_wait)
-		appendPQExpBuffer(cmdLine, " -w");
+		appendPQExpBufferStr(cmdLine, " -w");
 
 	/* Don't propagate a value from an environment variable. */
 	if (registration && wait_seconds_arg && wait_seconds != DEFAULT_WAIT)
 		appendPQExpBuffer(cmdLine, " -t %d", wait_seconds);
 
 	if (registration && silent_mode)
-		appendPQExpBuffer(cmdLine, " -s");
+		appendPQExpBufferStr(cmdLine, " -s");
 
 	if (post_opts)
 	{
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index ef8b53cd09..2c49b0ff36 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -425,7 +425,7 @@ buildDefaultACLCommands(const char *type, const char *nspname,
 
 	if (strlen(initacls) != 0 || strlen(initracls) != 0)
 	{
-		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
+		appendPQExpBufferStr(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
 		if (!buildACLCommands("", NULL, NULL, type,
 							  initacls, initracls, owner,
 							  prefix->data, remoteVersion, sql))
@@ -433,7 +433,7 @@ buildDefaultACLCommands(const char *type, const char *nspname,
 			destroyPQExpBuffer(prefix);
 			return false;
 		}
-		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
+		appendPQExpBufferStr(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
 	}
 
 	if (!buildACLCommands("", NULL, NULL, type,
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 40c77af540..0f40328310 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -554,8 +554,8 @@ RestoreArchive(Archive *AHX)
 							 */
 							if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
 							{
-								appendPQExpBuffer(ftStmt,
-												  "ALTER TABLE IF EXISTS");
+								appendPQExpBufferStr(ftStmt,
+													 "ALTER TABLE IF EXISTS");
 								dropStmt = dropStmt + 11;
 							}
 
@@ -4870,7 +4870,7 @@ CloneArchive(ArchiveHandle *AH)
 		 * any data to/from the database.
 		 */
 		initPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, PQdb(AH->connection));
 		pghost = PQhost(AH->connection);
 		pgport = PQport(AH->connection);
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 8af5c7bebd..9d33c86a3b 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -149,7 +149,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
 	}
 
 	initPQExpBuffer(&connstr);
-	appendPQExpBuffer(&connstr, "dbname=");
+	appendPQExpBufferStr(&connstr, "dbname=");
 	appendConnStrVal(&connstr, newdb);
 
 	do
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9f59cc74ee..ab5247441b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1205,7 +1205,7 @@ setup_connection(Archive *AH, const char *dumpencoding,
 	{
 		PQExpBuffer query = createPQExpBuffer();
 
-		appendPQExpBuffer(query, "SET TRANSACTION SNAPSHOT ");
+		appendPQExpBufferStr(query, "SET TRANSACTION SNAPSHOT ");
 		appendStringLiteralConn(query, AH->sync_snapshot_id, conn);
 		ExecuteSqlStatement(AH, query->data);
 		destroyPQExpBuffer(query);
@@ -1315,8 +1315,8 @@ expand_schema_name_patterns(Archive *fout,
 
 	for (cell = patterns->head; cell; cell = cell->next)
 	{
-		appendPQExpBuffer(query,
-						  "SELECT oid FROM pg_catalog.pg_namespace n\n");
+		appendPQExpBufferStr(query,
+							 "SELECT oid FROM pg_catalog.pg_namespace n\n");
 		processSQLNamePattern(GetConnection(fout), query, cell->val, false,
 							  false, NULL, "n.nspname", NULL, NULL);
 
@@ -3733,7 +3733,7 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
 	if (polinfo->polwithcheck != NULL)
 		appendPQExpBuffer(query, " WITH CHECK (%s)", polinfo->polwithcheck);
 
-	appendPQExpBuffer(query, ";\n");
+	appendPQExpBufferStr(query, ";\n");
 
 	appendPQExpBuffer(delqry, "DROP POLICY %s", fmtId(polinfo->polname));
 	appendPQExpBuffer(delqry, " ON %s;\n", fmtQualifiedDumpable(tbinfo));
@@ -4560,7 +4560,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
 						  init_acl_subquery->data,
 						  init_racl_subquery->data);
 
-		appendPQExpBuffer(query, ") ");
+		appendPQExpBufferStr(query, ") ");
 
 		destroyPQExpBuffer(acl_subquery);
 		destroyPQExpBuffer(racl_subquery);
@@ -5248,9 +5248,9 @@ getAccessMethods(Archive *fout, int *numAccessMethods)
 	query = createPQExpBuffer();
 
 	/* Select all access methods from pg_am table */
-	appendPQExpBuffer(query, "SELECT tableoid, oid, amname, amtype, "
-					  "amhandler::pg_catalog.regproc AS amhandler "
-					  "FROM pg_am");
+	appendPQExpBufferStr(query, "SELECT tableoid, oid, amname, amtype, "
+						 "amhandler::pg_catalog.regproc AS amhandler "
+						 "FROM pg_am");
 
 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 
@@ -8128,10 +8128,10 @@ getTransforms(Archive *fout, int *numTransforms)
 
 	query = createPQExpBuffer();
 
-	appendPQExpBuffer(query, "SELECT tableoid, oid, "
-					  "trftype, trflang, trffromsql::oid, trftosql::oid "
-					  "FROM pg_transform "
-					  "ORDER BY 3,4");
+	appendPQExpBufferStr(query, "SELECT tableoid, oid, "
+						 "trftype, trflang, trffromsql::oid, trftosql::oid "
+						 "FROM pg_transform "
+						 "ORDER BY 3,4");
 
 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 
@@ -8255,55 +8255,55 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 
 		resetPQExpBuffer(q);
 
-		appendPQExpBuffer(q,
-						  "SELECT\n"
-						  "a.attnum,\n"
-						  "a.attname,\n"
-						  "a.atttypmod,\n"
-						  "a.attstattarget,\n"
-						  "a.attstorage,\n"
-						  "t.typstorage,\n"
-						  "a.attnotnull,\n"
-						  "a.atthasdef,\n"
-						  "a.attisdropped,\n"
-						  "a.attlen,\n"
-						  "a.attalign,\n"
-						  "a.attislocal,\n"
-						  "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
+		appendPQExpBufferStr(q,
+							 "SELECT\n"
+							 "a.attnum,\n"
+							 "a.attname,\n"
+							 "a.atttypmod,\n"
+							 "a.attstattarget,\n"
+							 "a.attstorage,\n"
+							 "t.typstorage,\n"
+							 "a.attnotnull,\n"
+							 "a.atthasdef,\n"
+							 "a.attisdropped,\n"
+							 "a.attlen,\n"
+							 "a.attalign,\n"
+							 "a.attislocal,\n"
+							 "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
 
 		if (fout->remoteVersion >= 120000)
-			appendPQExpBuffer(q,
-							  "a.attgenerated,\n");
+			appendPQExpBufferStr(q,
+								 "a.attgenerated,\n");
 		else
-			appendPQExpBuffer(q,
-							  "'' AS attgenerated,\n");
+			appendPQExpBufferStr(q,
+								 "'' AS attgenerated,\n");
 
 		if (fout->remoteVersion >= 110000)
-			appendPQExpBuffer(q,
-							  "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
-							  "THEN a.attmissingval ELSE null END AS attmissingval,\n");
+			appendPQExpBufferStr(q,
+								 "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
+								 "THEN a.attmissingval ELSE null END AS attmissingval,\n");
 		else
-			appendPQExpBuffer(q,
-							  "NULL AS attmissingval,\n");
+			appendPQExpBufferStr(q,
+								 "NULL AS attmissingval,\n");
 
 		if (fout->remoteVersion >= 100000)
-			appendPQExpBuffer(q,
-							  "a.attidentity,\n");
+			appendPQExpBufferStr(q,
+								 "a.attidentity,\n");
 		else
-			appendPQExpBuffer(q,
-							  "'' AS attidentity,\n");
+			appendPQExpBufferStr(q,
+								 "'' AS attidentity,\n");
 
 		if (fout->remoteVersion >= 90200)
-			appendPQExpBuffer(q,
-							  "pg_catalog.array_to_string(ARRAY("
-							  "SELECT pg_catalog.quote_ident(option_name) || "
-							  "' ' || pg_catalog.quote_literal(option_value) "
-							  "FROM pg_catalog.pg_options_to_table(attfdwoptions) "
-							  "ORDER BY option_name"
-							  "), E',\n    ') AS attfdwoptions,\n");
+			appendPQExpBufferStr(q,
+								 "pg_catalog.array_to_string(ARRAY("
+								 "SELECT pg_catalog.quote_ident(option_name) || "
+								 "' ' || pg_catalog.quote_literal(option_value) "
+								 "FROM pg_catalog.pg_options_to_table(attfdwoptions) "
+								 "ORDER BY option_name"
+								 "), E',\n    ') AS attfdwoptions,\n");
 		else
-			appendPQExpBuffer(q,
-							  "'' AS attfdwoptions,\n");
+			appendPQExpBufferStr(q,
+								 "'' AS attfdwoptions,\n");
 
 		if (fout->remoteVersion >= 90100)
 		{
@@ -8312,20 +8312,20 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			 * collation is different from their type's default, we use a CASE
 			 * here to suppress uninteresting attcollations cheaply.
 			 */
-			appendPQExpBuffer(q,
-							  "CASE WHEN a.attcollation <> t.typcollation "
-							  "THEN a.attcollation ELSE 0 END AS attcollation,\n");
+			appendPQExpBufferStr(q,
+								 "CASE WHEN a.attcollation <> t.typcollation "
+								 "THEN a.attcollation ELSE 0 END AS attcollation,\n");
 		}
 		else
-			appendPQExpBuffer(q,
-							  "0 AS attcollation,\n");
+			appendPQExpBufferStr(q,
+								 "0 AS attcollation,\n");
 
 		if (fout->remoteVersion >= 90000)
-			appendPQExpBuffer(q,
-							  "array_to_string(a.attoptions, ', ') AS attoptions\n");
+			appendPQExpBufferStr(q,
+								 "array_to_string(a.attoptions, ', ') AS attoptions\n");
 		else
-			appendPQExpBuffer(q,
-							  "'' AS attoptions\n");
+			appendPQExpBufferStr(q,
+								 "'' AS attoptions\n");
 
 		/* need left join here to not fail on dropped columns ... */
 		appendPQExpBuffer(q,
@@ -12326,7 +12326,7 @@ dumpTransform(Archive *fout, TransformInfo *transform)
 	if (transform->trftosql)
 	{
 		if (transform->trffromsql)
-			appendPQExpBuffer(defqry, ", ");
+			appendPQExpBufferStr(defqry, ", ");
 
 		if (tosqlFuncInfo)
 		{
@@ -12344,7 +12344,7 @@ dumpTransform(Archive *fout, TransformInfo *transform)
 			pg_log_warning("bogus value in pg_transform.trftosql field");
 	}
 
-	appendPQExpBuffer(defqry, ");\n");
+	appendPQExpBufferStr(defqry, ");\n");
 
 	appendPQExpBuffer(labelq, "TRANSFORM FOR %s LANGUAGE %s",
 					  transformType, lanname);
@@ -12719,10 +12719,10 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 	switch (aminfo->amtype)
 	{
 		case AMTYPE_INDEX:
-			appendPQExpBuffer(q, "TYPE INDEX ");
+			appendPQExpBufferStr(q, "TYPE INDEX ");
 			break;
 		case AMTYPE_TABLE:
-			appendPQExpBuffer(q, "TYPE TABLE ");
+			appendPQExpBufferStr(q, "TYPE TABLE ");
 			break;
 		default:
 			pg_log_warning("invalid type \"%c\" of access method \"%s\"",
@@ -13428,23 +13428,23 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
 	qcollname = pg_strdup(fmtId(collinfo->dobj.name));
 
 	/* Get collation-specific details */
-	appendPQExpBuffer(query, "SELECT ");
+	appendPQExpBufferStr(query, "SELECT ");
 
 	if (fout->remoteVersion >= 100000)
-		appendPQExpBuffer(query,
-						  "collprovider, "
-						  "collversion, ");
+		appendPQExpBufferStr(query,
+							 "collprovider, "
+							 "collversion, ");
 	else
-		appendPQExpBuffer(query,
-						  "'c' AS collprovider, "
-						  "NULL AS collversion, ");
+		appendPQExpBufferStr(query,
+							 "'c' AS collprovider, "
+							 "NULL AS collversion, ");
 
 	if (fout->remoteVersion >= 120000)
-		appendPQExpBuffer(query,
-						  "collisdeterministic, ");
+		appendPQExpBufferStr(query,
+							 "collisdeterministic, ");
 	else
-		appendPQExpBuffer(query,
-						  "true AS collisdeterministic, ");
+		appendPQExpBufferStr(query,
+							 "true AS collisdeterministic, ");
 
 	appendPQExpBuffer(query,
 					  "collcollate, "
@@ -13660,7 +13660,7 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes)
 		appendPQExpBufferStr(&buf, agginfo->aggfn.dobj.name);
 
 	if (agginfo->aggfn.nargs == 0)
-		appendPQExpBuffer(&buf, "(*)");
+		appendPQExpBufferStr(&buf, "(*)");
 	else
 	{
 		appendPQExpBufferChar(&buf, '(');
@@ -14878,13 +14878,13 @@ dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
 	 */
 	if (strlen(initacls) != 0 || strlen(initracls) != 0)
 	{
-		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
+		appendPQExpBufferStr(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
 		if (!buildACLCommands(name, subname, nspname, type,
 							  initacls, initracls, owner,
 							  "", fout->remoteVersion, sql))
 			fatal("could not parse initial GRANT ACL list (%s) or initial REVOKE ACL list (%s) for object \"%s\" (%s)",
 				  initacls, initracls, name, type);
-		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
+		appendPQExpBufferStr(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
 	}
 
 	if (!buildACLCommands(name, subname, nspname, type,
@@ -16593,7 +16593,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 			}
 
 			if (indxinfo->indnkeyattrs < indxinfo->indnattrs)
-				appendPQExpBuffer(q, ") INCLUDE (");
+				appendPQExpBufferStr(q, ") INCLUDE (");
 
 			for (k = indxinfo->indnkeyattrs; k < indxinfo->indnattrs; k++)
 			{
@@ -16990,9 +16990,9 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
 						  "ALTER COLUMN %s ADD GENERATED ",
 						  fmtId(owning_tab->attnames[tbinfo->owning_col - 1]));
 		if (owning_tab->attidentity[tbinfo->owning_col - 1] == ATTRIBUTE_IDENTITY_ALWAYS)
-			appendPQExpBuffer(query, "ALWAYS");
+			appendPQExpBufferStr(query, "ALWAYS");
 		else if (owning_tab->attidentity[tbinfo->owning_col - 1] == ATTRIBUTE_IDENTITY_BY_DEFAULT)
-			appendPQExpBuffer(query, "BY DEFAULT");
+			appendPQExpBufferStr(query, "BY DEFAULT");
 		appendPQExpBuffer(query, " AS IDENTITY (\n    SEQUENCE NAME %s\n",
 						  fmtQualifiedDumpable(tbinfo));
 	}
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index ea4ac91c00..df7e6c0bda 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1425,8 +1425,8 @@ expand_dbname_patterns(PGconn *conn,
 
 	for (SimpleStringListCell *cell = patterns->head; cell; cell = cell->next)
 	{
-		appendPQExpBuffer(query,
-						  "SELECT datname FROM pg_catalog.pg_database n\n");
+		appendPQExpBufferStr(query,
+							 "SELECT datname FROM pg_catalog.pg_database n\n");
 		processSQLNamePattern(conn, query, cell->val, false,
 							  false, NULL, "datname", NULL, NULL);
 
diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index 0ffe171a05..c1429fe4bf 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -42,7 +42,7 @@ generate_old_dump(void)
 					escaped_connstr;
 
 		initPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, old_db->db_name);
 		initPQExpBuffer(&escaped_connstr);
 		appendShellString(&escaped_connstr, connstr.data);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a3280eeb18..e5dbd75ee0 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2972,7 +2972,7 @@ do_connect(enum trivalue reuse_previous_specification,
 	if (!dbname && reuse_previous)
 	{
 		initPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, PQdb(o_conn));
 		dbname = connstr.data;
 		/* has_connection_string=true would be a dead store */
@@ -4553,7 +4553,7 @@ lookup_object_oid(EditableObjectType obj_type, const char *desc,
 			 */
 			appendPQExpBufferStr(query, "SELECT ");
 			appendStringLiteralConn(query, desc, pset.db);
-			appendPQExpBuffer(query, "::pg_catalog.regclass::pg_catalog.oid");
+			appendPQExpBufferStr(query, "::pg_catalog.regclass::pg_catalog.oid");
 			break;
 	}
 
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 97167d2c4b..9b11dd1278 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2115,8 +2115,8 @@ describeOneTableDetails(const char *schemaname,
 						  "  pg_catalog.pg_get_expr(c.relpartbound, inhrelid)");
 		/* If verbose, also request the partition constraint definition */
 		if (verbose)
-			appendPQExpBuffer(&buf,
-							  ",\n  pg_catalog.pg_get_partition_constraintdef(inhrelid)");
+			appendPQExpBufferStr(&buf,
+								 ",\n  pg_catalog.pg_get_partition_constraintdef(inhrelid)");
 		appendPQExpBuffer(&buf,
 						  "\nFROM pg_catalog.pg_class c"
 						  " JOIN pg_catalog.pg_inherits i"
@@ -2203,9 +2203,9 @@ describeOneTableDetails(const char *schemaname,
 								 "  false AS condeferrable, false AS condeferred,\n");
 
 		if (pset.sversion >= 90400)
-			appendPQExpBuffer(&buf, "i.indisreplident,\n");
+			appendPQExpBufferStr(&buf, "i.indisreplident,\n");
 		else
-			appendPQExpBuffer(&buf, "false AS indisreplident,\n");
+			appendPQExpBufferStr(&buf, "false AS indisreplident,\n");
 
 		appendPQExpBuffer(&buf, "  a.amname, c2.relname, "
 						  "pg_catalog.pg_get_expr(i.indpred, i.indrelid, true)\n"
@@ -2263,7 +2263,7 @@ describeOneTableDetails(const char *schemaname,
 				appendPQExpBufferStr(&tmpbuf, _(", initially deferred"));
 
 			if (strcmp(indisreplident, "t") == 0)
-				appendPQExpBuffer(&tmpbuf, _(", replica identity"));
+				appendPQExpBufferStr(&tmpbuf, _(", replica identity"));
 
 			printTableAddFooter(&cont, tmpbuf.data);
 			add_tablespace_footer(&cont, tableinfo.relkind,
@@ -2374,7 +2374,7 @@ describeOneTableDetails(const char *schemaname,
 						appendPQExpBufferStr(&buf, " INVALID");
 
 					if (strcmp(PQgetvalue(result, i, 10), "t") == 0)
-						appendPQExpBuffer(&buf, " REPLICA IDENTITY");
+						appendPQExpBufferStr(&buf, " REPLICA IDENTITY");
 
 					printTableAddFooter(&cont, buf.data);
 
@@ -2457,8 +2457,8 @@ describeOneTableDetails(const char *schemaname,
 								  oid);
 
 				if (pset.sversion >= 120000)
-					appendPQExpBuffer(&buf, "     AND conparentid = 0\n");
-				appendPQExpBuffer(&buf, "ORDER BY conname");
+					appendPQExpBufferStr(&buf, "     AND conparentid = 0\n");
+				appendPQExpBufferStr(&buf, "ORDER BY conname");
 			}
 
 			result = PSQLexec(buf.data);
@@ -2556,11 +2556,11 @@ describeOneTableDetails(const char *schemaname,
 		{
 			printfPQExpBuffer(&buf, "SELECT pol.polname,");
 			if (pset.sversion >= 100000)
-				appendPQExpBuffer(&buf,
-								  " pol.polpermissive,\n");
+				appendPQExpBufferStr(&buf,
+									 " pol.polpermissive,\n");
 			else
-				appendPQExpBuffer(&buf,
-								  " 't' as polpermissive,\n");
+				appendPQExpBufferStr(&buf,
+									 " 't' as polpermissive,\n");
 			appendPQExpBuffer(&buf,
 							  "  CASE WHEN pol.polroles = '{0}' THEN NULL ELSE pg_catalog.array_to_string(array(select rolname from pg_catalog.pg_roles where oid = any (pol.polroles) order by 1),',') END,\n"
 							  "  pg_catalog.pg_get_expr(pol.polqual, pol.polrelid),\n"
@@ -2608,7 +2608,7 @@ describeOneTableDetails(const char *schemaname,
 								  PQgetvalue(result, i, 0));
 
 				if (*(PQgetvalue(result, i, 1)) == 'f')
-					appendPQExpBuffer(&buf, " AS RESTRICTIVE");
+					appendPQExpBufferStr(&buf, " AS RESTRICTIVE");
 
 				if (!PQgetisnull(result, i, 5))
 					appendPQExpBuffer(&buf, " FOR %s",
@@ -2913,12 +2913,12 @@ describeOneTableDetails(const char *schemaname,
 						   "t.tgconstraint <> 0 AS tgisinternal" :
 						   "false AS tgisinternal"), oid);
 		if (pset.sversion >= 110000)
-			appendPQExpBuffer(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D') \n"
-							  "    OR EXISTS (SELECT 1 FROM pg_catalog.pg_depend WHERE objid = t.oid \n"
-							  "        AND refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass))");
+			appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D') \n"
+								 "    OR EXISTS (SELECT 1 FROM pg_catalog.pg_depend WHERE objid = t.oid \n"
+								 "        AND refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass))");
 		else if (pset.sversion >= 90000)
 			/* display/warn about disabled internal triggers */
-			appendPQExpBuffer(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))");
+			appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))");
 		else if (pset.sversion >= 80300)
 			appendPQExpBufferStr(&buf, "(t.tgconstraint = 0 OR (t.tgconstraint <> 0 AND t.tgenabled = 'D'))");
 		else
@@ -3909,33 +3909,33 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 	{
 		if (pset.sversion < 120000)
 		{
-			appendPQExpBuffer(&buf,
-							  ",\n     LATERAL (WITH RECURSIVE d\n"
-							  "                AS (SELECT inhrelid AS oid, 1 AS level\n"
-							  "                      FROM pg_catalog.pg_inherits\n"
-							  "                     WHERE inhparent = c.oid\n"
-							  "                    UNION ALL\n"
-							  "                    SELECT inhrelid, level + 1\n"
-							  "                      FROM pg_catalog.pg_inherits i\n"
-							  "                           JOIN d ON i.inhparent = d.oid)\n"
-							  "                SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
-							  "d.oid))) AS tps,\n"
-							  "                       pg_catalog.pg_size_pretty(sum("
-							  "\n             CASE WHEN d.level = 1"
-							  " THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
-							  "               FROM d) s");
+			appendPQExpBufferStr(&buf,
+								 ",\n     LATERAL (WITH RECURSIVE d\n"
+								 "                AS (SELECT inhrelid AS oid, 1 AS level\n"
+								 "                      FROM pg_catalog.pg_inherits\n"
+								 "                     WHERE inhparent = c.oid\n"
+								 "                    UNION ALL\n"
+								 "                    SELECT inhrelid, level + 1\n"
+								 "                      FROM pg_catalog.pg_inherits i\n"
+								 "                           JOIN d ON i.inhparent = d.oid)\n"
+								 "                SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
+								 "d.oid))) AS tps,\n"
+								 "                       pg_catalog.pg_size_pretty(sum("
+								 "\n             CASE WHEN d.level = 1"
+								 " THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
+								 "               FROM d) s");
 		}
 		else
 		{
 			/* PostgreSQL 12 has pg_partition_tree function */
-			appendPQExpBuffer(&buf,
-							  ",\n     LATERAL (SELECT pg_catalog.pg_size_pretty(sum("
-							  "\n                 CASE WHEN ppt.isleaf AND ppt.level = 1"
-							  "\n                      THEN pg_catalog.pg_table_size(ppt.relid)"
-							  " ELSE 0 END)) AS dps"
-							  ",\n                     pg_catalog.pg_size_pretty(sum("
-							  "pg_catalog.pg_table_size(ppt.relid))) AS tps"
-							  "\n              FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
+			appendPQExpBufferStr(&buf,
+								 ",\n     LATERAL (SELECT pg_catalog.pg_size_pretty(sum("
+								 "\n                 CASE WHEN ppt.isleaf AND ppt.level = 1"
+								 "\n                      THEN pg_catalog.pg_table_size(ppt.relid)"
+								 " ELSE 0 END)) AS dps"
+								 ",\n                     pg_catalog.pg_size_pretty(sum("
+								 "pg_catalog.pg_table_size(ppt.relid))) AS tps"
+								 "\n              FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
 		}
 	}
 
@@ -3977,7 +3977,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 		return false;
 
 	initPQExpBuffer(&title);
-	appendPQExpBuffer(&title, "%s", tabletitle);
+	appendPQExpBufferStr(&title, tabletitle);
 
 	myopt.nullPrint = NULL;
 	myopt.title = title.data;
@@ -4541,8 +4541,8 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
 						  gettext_noop("Description"));
 	}
 
-	appendPQExpBuffer(&buf,
-					  "\nFROM pg_catalog.pg_namespace n\n");
+	appendPQExpBufferStr(&buf,
+						 "\nFROM pg_catalog.pg_namespace n\n");
 
 	if (!showSystem && !pattern)
 		appendPQExpBufferStr(&buf,
@@ -5742,10 +5742,10 @@ describePublications(const char *pattern)
 					  "  pg_catalog.pg_get_userbyid(pubowner) AS owner,\n"
 					  "  puballtables, pubinsert, pubupdate, pubdelete");
 	if (has_pubtruncate)
-		appendPQExpBuffer(&buf,
-						  ", pubtruncate");
-	appendPQExpBuffer(&buf,
-					  "\nFROM pg_catalog.pg_publication\n");
+		appendPQExpBufferStr(&buf,
+							 ", pubtruncate");
+	appendPQExpBufferStr(&buf,
+						 "\nFROM pg_catalog.pg_publication\n");
 
 	processSQLNamePattern(pset.db, &buf, pattern, false, false,
 						  NULL, "pubname", NULL,
diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 15fff91e16..ae0facd5a7 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -254,7 +254,7 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
 		}
 
 		resetPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, dbname);
 
 		cluster_one_database(connstr.data, verbose, NULL,
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index bec57589d3..c9293e2262 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -371,7 +371,7 @@ reindex_all_databases(const char *maintenance_db,
 		}
 
 		resetPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, dbname);
 
 		reindex_one_database(NULL, connstr.data, "DATABASE", host,
@@ -396,14 +396,14 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
 
 	initPQExpBuffer(&sql);
 
-	appendPQExpBuffer(&sql, "REINDEX");
+	appendPQExpBufferStr(&sql, "REINDEX");
 
 	if (verbose)
-		appendPQExpBuffer(&sql, " (VERBOSE)");
+		appendPQExpBufferStr(&sql, " (VERBOSE)");
 
 	appendPQExpBufferStr(&sql, " SYSTEM ");
 	if (concurrently)
-		appendPQExpBuffer(&sql, "CONCURRENTLY ");
+		appendPQExpBufferStr(&sql, "CONCURRENTLY ");
 	appendPQExpBufferStr(&sql, fmtId(PQdb(conn)));
 	appendPQExpBufferChar(&sql, ';');
 
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index df2a315f95..3bcd14b4dc 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -477,16 +477,16 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 
 		if (!tables_listed)
 		{
-			appendPQExpBuffer(&catalog_query,
-							  "WITH listed_tables (table_oid, column_list) "
-							  "AS (\n  VALUES (");
+			appendPQExpBufferStr(&catalog_query,
+								 "WITH listed_tables (table_oid, column_list) "
+								 "AS (\n  VALUES (");
 			tables_listed = true;
 		}
 		else
-			appendPQExpBuffer(&catalog_query, ",\n  (");
+			appendPQExpBufferStr(&catalog_query, ",\n  (");
 
 		appendStringLiteralConn(&catalog_query, just_table, conn);
-		appendPQExpBuffer(&catalog_query, "::pg_catalog.regclass, ");
+		appendPQExpBufferStr(&catalog_query, "::pg_catalog.regclass, ");
 
 		if (just_columns && just_columns[0] != '\0')
 			appendStringLiteralConn(&catalog_query, just_columns, conn);
@@ -500,24 +500,24 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 
 	/* Finish formatting the CTE */
 	if (tables_listed)
-		appendPQExpBuffer(&catalog_query, "\n)\n");
+		appendPQExpBufferStr(&catalog_query, "\n)\n");
 
-	appendPQExpBuffer(&catalog_query, "SELECT c.relname, ns.nspname");
+	appendPQExpBufferStr(&catalog_query, "SELECT c.relname, ns.nspname");
 
 	if (tables_listed)
-		appendPQExpBuffer(&catalog_query, ", listed_tables.column_list");
+		appendPQExpBufferStr(&catalog_query, ", listed_tables.column_list");
 
-	appendPQExpBuffer(&catalog_query,
-					  " FROM pg_catalog.pg_class c\n"
-					  " JOIN pg_catalog.pg_namespace ns"
-					  " ON c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n"
-					  " LEFT JOIN pg_catalog.pg_class t"
-					  " ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n");
+	appendPQExpBufferStr(&catalog_query,
+						 " FROM pg_catalog.pg_class c\n"
+						 " JOIN pg_catalog.pg_namespace ns"
+						 " ON c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n"
+						 " LEFT JOIN pg_catalog.pg_class t"
+						 " ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n");
 
 	/* Used to match the tables listed by the user */
 	if (tables_listed)
-		appendPQExpBuffer(&catalog_query, " JOIN listed_tables"
-						  " ON listed_tables.table_oid OPERATOR(pg_catalog.=) c.oid\n");
+		appendPQExpBufferStr(&catalog_query, " JOIN listed_tables"
+							 " ON listed_tables.table_oid OPERATOR(pg_catalog.=) c.oid\n");
 
 	/*
 	 * If no tables were listed, filter for the relevant relation types.  If
@@ -527,9 +527,9 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 	 */
 	if (!tables_listed)
 	{
-		appendPQExpBuffer(&catalog_query, " WHERE c.relkind OPERATOR(pg_catalog.=) ANY (array["
-						  CppAsString2(RELKIND_RELATION) ", "
-						  CppAsString2(RELKIND_MATVIEW) "])\n");
+		appendPQExpBufferStr(&catalog_query, " WHERE c.relkind OPERATOR(pg_catalog.=) ANY (array["
+							 CppAsString2(RELKIND_RELATION) ", "
+							 CppAsString2(RELKIND_MATVIEW) "])\n");
 		has_where = true;
 	}
 
@@ -568,7 +568,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 	 * Execute the catalog query.  We use the default search_path for this
 	 * query for consistency with table lookups done elsewhere by the user.
 	 */
-	appendPQExpBuffer(&catalog_query, " ORDER BY c.relpages DESC;");
+	appendPQExpBufferStr(&catalog_query, " ORDER BY c.relpages DESC;");
 	executeCommand(conn, "RESET search_path;", progname, echo);
 	res = executeQuery(conn, catalog_query.data, progname, echo);
 	termPQExpBuffer(&catalog_query);
@@ -775,7 +775,7 @@ vacuum_all_databases(vacuumingOptions *vacopts,
 			for (i = 0; i < PQntuples(result); i++)
 			{
 				resetPQExpBuffer(&connstr);
-				appendPQExpBuffer(&connstr, "dbname=");
+				appendPQExpBufferStr(&connstr, "dbname=");
 				appendConnStrVal(&connstr, PQgetvalue(result, i, 0));
 
 				vacuum_one_database(connstr.data, vacopts,
@@ -792,7 +792,7 @@ vacuum_all_databases(vacuumingOptions *vacopts,
 		for (i = 0; i < PQntuples(result); i++)
 		{
 			resetPQExpBuffer(&connstr);
-			appendPQExpBuffer(&connstr, "dbname=");
+			appendPQExpBufferStr(&connstr, "dbname=");
 			appendConnStrVal(&connstr, PQgetvalue(result, i, 0));
 
 			vacuum_one_database(connstr.data, vacopts,
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 58610dbf57..758e5bc96d 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -625,10 +625,10 @@ appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname)
 		PQExpBufferData connstr;
 
 		initPQExpBuffer(&connstr);
-		appendPQExpBuffer(&connstr, "dbname=");
+		appendPQExpBufferStr(&connstr, "dbname=");
 		appendConnStrVal(&connstr, dbname);
 
-		appendPQExpBuffer(buf, "-reuse-previous=on ");
+		appendPQExpBufferStr(buf, "-reuse-previous=on ");
 
 		/*
 		 * As long as the name does not contain a newline, SQL identifier
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 719570a45c..858adf15f4 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -346,7 +346,7 @@ build_client_first_message(fe_scram_state *state)
 	if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0)
 	{
 		Assert(conn->ssl_in_use);
-		appendPQExpBuffer(&buf, "p=tls-server-end-point");
+		appendPQExpBufferStr(&buf, "p=tls-server-end-point");
 	}
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
 	else if (conn->ssl_in_use)
@@ -354,7 +354,7 @@ build_client_first_message(fe_scram_state *state)
 		/*
 		 * Client supports channel binding, but thinks the server does not.
 		 */
-		appendPQExpBuffer(&buf, "y");
+		appendPQExpBufferChar(&buf, 'y');
 	}
 #endif
 	else
@@ -362,7 +362,7 @@ build_client_first_message(fe_scram_state *state)
 		/*
 		 * Client does not support channel binding.
 		 */
-		appendPQExpBuffer(&buf, "n");
+		appendPQExpBufferChar(&buf, 'n');
 	}
 
 	if (PQExpBufferDataBroken(buf))
@@ -437,7 +437,7 @@ build_client_final_message(fe_scram_state *state)
 			return NULL;
 		}
 
-		appendPQExpBuffer(&buf, "c=");
+		appendPQExpBufferStr(&buf, "c=");
 
 		/* p=type,, */
 		cbind_header_len = strlen("p=tls-server-end-point,,");
@@ -475,10 +475,10 @@ build_client_final_message(fe_scram_state *state)
 	}
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
 	else if (conn->ssl_in_use)
-		appendPQExpBuffer(&buf, "c=eSws");	/* base64 of "y,," */
+		appendPQExpBufferStr(&buf, "c=eSws");	/* base64 of "y,," */
 #endif
 	else
-		appendPQExpBuffer(&buf, "c=biws");	/* base64 of "n,," */
+		appendPQExpBufferStr(&buf, "c=biws");	/* base64 of "n,," */
 
 	if (PQExpBufferDataBroken(buf))
 		goto oom_error;
@@ -496,7 +496,7 @@ build_client_final_message(fe_scram_state *state)
 						   state->client_final_message_without_proof,
 						   client_proof);
 
-	appendPQExpBuffer(&buf, ",p=");
+	appendPQExpBufferStr(&buf, ",p=");
 	if (!enlargePQExpBuffer(&buf, pg_b64_enc_len(SCRAM_KEY_LEN)))
 		goto oom_error;
 	buf.len += pg_b64_encode((char *) client_proof,
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e58fa6742a..64370fe61d 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2774,8 +2774,8 @@ keep_going:						/* We will come back to here until there is
 				}
 				else if (!conn->gctx && conn->gssencmode[0] == 'r')
 				{
-					appendPQExpBuffer(&conn->errorMessage,
-									  libpq_gettext("GSSAPI encryption required, but was impossible (possibly no ccache, no server support, or using a local socket)\n"));
+					appendPQExpBufferStr(&conn->errorMessage,
+										 libpq_gettext("GSSAPI encryption required, but was impossible (possibly no ccache, no server support, or using a local socket)\n"));
 					goto error_return;
 				}
 #endif
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 467563d7a4..bbba48bc8b 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -996,7 +996,7 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
 	/* If we couldn't allocate a PGresult, just say "out of memory" */
 	if (res == NULL)
 	{
-		appendPQExpBuffer(msg, libpq_gettext("out of memory\n"));
+		appendPQExpBufferStr(msg, libpq_gettext("out of memory\n"));
 		return;
 	}
 
@@ -1009,7 +1009,7 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
 		if (res->errMsg && res->errMsg[0])
 			appendPQExpBufferStr(msg, res->errMsg);
 		else
-			appendPQExpBuffer(msg, libpq_gettext("no error message available\n"));
+			appendPQExpBufferStr(msg, libpq_gettext("no error message available\n"));
 		return;
 	}
 
-- 
2.20.1

From 0004249c62a4afba1121282687f2b3d773241b54 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Sun, 28 Apr 2019 13:42:04 +1200
Subject: [PATCH 2/4] Use appendStringInfoString instead of appendStringInfo,
 where possible

If the call does not require any printf type formatting then there is no
point in using appendStringInfo, doing so just adds overhead and leaves
bad examples in the code.
---
 contrib/postgres_fdw/deparse.c         | 8 ++++----
 contrib/test_decoding/test_decoding.c  | 4 ++--
 src/backend/access/rmgrdesc/heapdesc.c | 4 ++--
 src/backend/commands/explain.c         | 2 +-
 src/backend/utils/adt/ruleutils.c      | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 6da4c834bf..b951b11592 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1531,7 +1531,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
 			{
 				Assert(fpinfo->jointype == JOIN_INNER);
 				Assert(fpinfo->joinclauses == NIL);
-				appendStringInfo(buf, "%s", join_sql_o.data);
+				appendStringInfoString(buf, join_sql_o.data);
 				return;
 			}
 		}
@@ -1552,7 +1552,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
 			{
 				Assert(fpinfo->jointype == JOIN_INNER);
 				Assert(fpinfo->joinclauses == NIL);
-				appendStringInfo(buf, "%s", join_sql_i.data);
+				appendStringInfoString(buf, join_sql_i.data);
 				return;
 			}
 		}
@@ -1861,7 +1861,7 @@ deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
 	{
 		List	   *ignore_conds = NIL;
 
-		appendStringInfo(buf, " FROM ");
+		appendStringInfoString(buf, " FROM ");
 		deparseFromExprForRel(buf, root, foreignrel, true, rtindex,
 							  &ignore_conds, params_list);
 		remote_conds = list_concat(remote_conds, ignore_conds);
@@ -1944,7 +1944,7 @@ deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
 	{
 		List	   *ignore_conds = NIL;
 
-		appendStringInfo(buf, " USING ");
+		appendStringInfoString(buf, " USING ");
 		deparseFromExprForRel(buf, root, foreignrel, true, rtindex,
 							  &ignore_conds, params_list);
 		remote_conds = list_concat(remote_conds, ignore_conds);
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c
index 33955a478e..a0ded67b9a 100644
--- a/contrib/test_decoding/test_decoding.c
+++ b/contrib/test_decoding/test_decoding.c
@@ -518,9 +518,9 @@ pg_decode_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
 		|| change->data.truncate.cascade)
 	{
 		if (change->data.truncate.restart_seqs)
-			appendStringInfo(ctx->out, " restart_seqs");
+			appendStringInfoString(ctx->out, " restart_seqs");
 		if (change->data.truncate.cascade)
-			appendStringInfo(ctx->out, " cascade");
+			appendStringInfoString(ctx->out, " cascade");
 	}
 	else
 		appendStringInfoString(ctx->out, " (no-flags)");
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index eff529f990..f25ebcda99 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -86,9 +86,9 @@ heap_desc(StringInfo buf, XLogReaderState *record)
 		int			i;
 
 		if (xlrec->flags & XLH_TRUNCATE_CASCADE)
-			appendStringInfo(buf, "cascade ");
+			appendStringInfoString(buf, "cascade ");
 		if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS)
-			appendStringInfo(buf, "restart_seqs ");
+			appendStringInfoString(buf, "restart_seqs ");
 		appendStringInfo(buf, "nrelids %u relids", xlrec->nrelids);
 		for (i = 0; i < xlrec->nrelids; i++)
 			appendStringInfo(buf, " %u", xlrec->relids[i]);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 92969636b7..dff2ed3f97 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -822,7 +822,7 @@ ExplainPrintJIT(ExplainState *es, int jit_flags,
 		if (for_workers)
 			appendStringInfo(es->str, "JIT for worker %u:\n", worker_num);
 		else
-			appendStringInfo(es->str, "JIT:\n");
+			appendStringInfoString(es->str, "JIT:\n");
 		es->indent += 1;
 
 		ExplainPropertyInteger("Functions", NULL, ji->created_functions, es);
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 9dda4820af..421b622b5e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1723,7 +1723,7 @@ pg_get_partkeydef_worker(Oid relid, int prettyFlags,
 	{
 		case PARTITION_STRATEGY_HASH:
 			if (!attrsOnly)
-				appendStringInfo(&buf, "HASH");
+				appendStringInfoString(&buf, "HASH");
 			break;
 		case PARTITION_STRATEGY_LIST:
 			if (!attrsOnly)
-- 
2.20.1

From 9544edaece51420e24520df13a651d56e3bdf150 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Sun, 28 Apr 2019 20:56:58 +1200
Subject: [PATCH 4/4] Add appendStringInfoStringInfo to append one StringInfo
 to another

This could be done previously with appendStringInfoString(t, s.data) or with
appendBinaryStringInfo(t, s.data, s.len), however the new function is faster
than the former and neater than the latter.
---
 contrib/postgres_fdw/deparse.c      |  4 ++--
 src/backend/executor/execMain.c     |  2 +-
 src/backend/storage/lmgr/deadlock.c |  2 +-
 src/backend/utils/adt/ri_triggers.c |  4 ++--
 src/backend/utils/adt/ruleutils.c   | 10 +++++-----
 src/backend/utils/adt/xml.c         | 10 +++++-----
 src/include/lib/stringinfo.h        | 10 ++++++++++
 7 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index b951b11592..7724dbf8a7 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1531,7 +1531,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
 			{
 				Assert(fpinfo->jointype == JOIN_INNER);
 				Assert(fpinfo->joinclauses == NIL);
-				appendStringInfoString(buf, join_sql_o.data);
+				appendStringInfoStringInfo(buf, &join_sql_o);
 				return;
 			}
 		}
@@ -1552,7 +1552,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
 			{
 				Assert(fpinfo->jointype == JOIN_INNER);
 				Assert(fpinfo->joinclauses == NIL);
-				appendStringInfoString(buf, join_sql_i.data);
+				appendStringInfoStringInfo(buf, &join_sql_i);
 				return;
 			}
 		}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 8c8528b134..49f66fb021 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2309,7 +2309,7 @@ ExecBuildSlotValueDescription(Oid reloid,
 	if (!table_perm)
 	{
 		appendStringInfoString(&collist, ") = ");
-		appendStringInfoString(&collist, buf.data);
+		appendStringInfoStringInfo(&collist, &buf);
 
 		return collist.data;
 	}
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index 9abc9d778f..14a47b9e66 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -1115,7 +1115,7 @@ DeadLockReport(void)
 	}
 
 	/* Duplicate all the above for the server ... */
-	appendStringInfoString(&logbuf, clientbuf.data);
+	appendStringInfoStringInfo(&logbuf, &clientbuf);
 
 	/* ... and add info about query strings */
 	for (i = 0; i < nDeadlockDetails; i++)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 44a6eef5bb..5b4872db09 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -927,7 +927,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
 			queryoids[i] = pk_type;
 			queryoids[j] = pk_type;
 		}
-		appendStringInfoString(&querybuf, qualbuf.data);
+		appendStringInfoStringInfo(&querybuf, &qualbuf);
 
 		/* Prepare and save the plan */
 		qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys * 2, queryoids,
@@ -1106,7 +1106,7 @@ ri_set(TriggerData *trigdata, bool is_set_null)
 			qualsep = "AND";
 			queryoids[i] = pk_type;
 		}
-		appendStringInfoString(&querybuf, qualbuf.data);
+		appendStringInfoStringInfo(&querybuf, &qualbuf);
 
 		/* Prepare and save the plan */
 		qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 421b622b5e..8ee50494e1 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2804,9 +2804,9 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
 		appendStringInfoChar(&dq, 'x');
 	appendStringInfoChar(&dq, '$');
 
-	appendStringInfoString(&buf, dq.data);
+	appendStringInfoStringInfo(&buf, &dq);
 	appendStringInfoString(&buf, prosrc);
-	appendStringInfoString(&buf, dq.data);
+	appendStringInfoStringInfo(&buf, &dq);
 
 	appendStringInfoChar(&buf, '\n');
 
@@ -2930,7 +2930,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
 		appendStringInfoString(&rbuf, format_type_be(proc->prorettype));
 	}
 
-	appendStringInfoString(buf, rbuf.data);
+	appendStringInfoStringInfo(buf, &rbuf);
 }
 
 /*
@@ -5684,7 +5684,7 @@ get_target_list(List *targetList, deparse_context *context,
 		}
 
 		/* Add the new field */
-		appendStringInfoString(buf, targetbuf.data);
+		appendStringInfoStringInfo(buf, &targetbuf);
 	}
 
 	/* clean up */
@@ -9989,7 +9989,7 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context)
 			}
 
 			/* Add the new item */
-			appendStringInfoString(buf, itembuf.data);
+			appendStringInfoStringInfo(buf, &itembuf);
 
 			/* clean up */
 			pfree(itembuf.data);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index d43c3055f3..2a3f5403cf 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -559,7 +559,7 @@ xmlconcat(List *args)
 					   0,
 					   global_standalone);
 
-		appendStringInfoString(&buf2, buf.data);
+		appendStringInfoStringInfo(&buf2, &buf);
 		buf = buf2;
 	}
 
@@ -1879,7 +1879,7 @@ xml_errorHandler(void *data, xmlErrorPtr error)
 	if (xmlerrcxt->strictness == PG_XML_STRICTNESS_LEGACY)
 	{
 		appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
-		appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+		appendStringInfoStringInfo(&xmlerrcxt->err_buf, errorBuf);
 
 		pfree(errorBuf->data);
 		pfree(errorBuf);
@@ -1897,7 +1897,7 @@ xml_errorHandler(void *data, xmlErrorPtr error)
 	if (level >= XML_ERR_ERROR)
 	{
 		appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
-		appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+		appendStringInfoStringInfo(&xmlerrcxt->err_buf, errorBuf);
 
 		xmlerrcxt->err_occurred = true;
 	}
@@ -2874,7 +2874,7 @@ schema_to_xml_internal(Oid nspid, const char *xmlschema, bool nulls,
 		subres = table_to_xml_internal(relid, NULL, nulls, tableforest,
 									   targetns, false);
 
-		appendStringInfoString(result, subres->data);
+		appendStringInfoStringInfo(result, subres);
 		appendStringInfoChar(result, '\n');
 	}
 
@@ -3049,7 +3049,7 @@ database_to_xml_internal(const char *xmlschema, bool nulls,
 		subres = schema_to_xml_internal(nspid, NULL, nulls,
 										tableforest, targetns, false);
 
-		appendStringInfoString(result, subres->data);
+		appendStringInfoStringInfo(result, subres);
 		appendStringInfoChar(result, '\n');
 	}
 
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 2e5aedb1a3..3fad5c5fc1 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -125,6 +125,16 @@ appendStringInfoString(StringInfo str, const char *s)
 	appendBinaryStringInfo(str, s, strlen(s));
 }
 
+/*------------------------
+ * appendStringInfoStringInfo
+ * Append the 'src' StringInfo to 'str'.
+ */
+static inline void
+appendStringInfoStringInfo(StringInfo str, StringInfo src)
+{
+	appendBinaryStringInfo(str, src->data, src->len);
+}
+
 /*------------------------
  * appendStringInfoChar
  * Append a single byte to str.
-- 
2.20.1

From 9f56571b24f72de928ddf8e05eabb1748d20f8b2 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Sun, 28 Apr 2019 14:15:05 +1200
Subject: [PATCH 3/4] Inline appendStringInfoString

Making this inline gives the compiler flexibility to optimize away strlen
calls when appendStringInfoString is called with a string constant, to which
it seemingly is, most of the time.
---
 src/backend/lib/stringinfo.c | 12 ------------
 src/bin/pg_waldump/compat.c  |  2 +-
 src/include/lib/stringinfo.h | 25 +++++++++++++++----------
 3 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c
index 99c83c1549..74ba478c0d 100644
--- a/src/backend/lib/stringinfo.c
+++ b/src/backend/lib/stringinfo.c
@@ -153,18 +153,6 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
 	return (int) nprinted;
 }
 
-/*
- * appendStringInfoString
- *
- * Append a null-terminated string to str.
- * Like appendStringInfo(str, "%s", s) but faster.
- */
-void
-appendStringInfoString(StringInfo str, const char *s)
-{
-	appendBinaryStringInfo(str, s, strlen(s));
-}
-
 /*
  * appendStringInfoChar
  *
diff --git a/src/bin/pg_waldump/compat.c b/src/bin/pg_waldump/compat.c
index 0e0dca7d1a..a22a0e88a7 100644
--- a/src/bin/pg_waldump/compat.c
+++ b/src/bin/pg_waldump/compat.c
@@ -79,7 +79,7 @@ appendStringInfo(StringInfo str, const char *fmt,...)
 }
 
 void
-appendStringInfoString(StringInfo str, const char *string)
+appendBinaryStringInfo(StringInfo str, const char *string, int datalen)
 {
 	appendStringInfo(str, "%s", string);
 }
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index c4842778c5..2e5aedb1a3 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -105,12 +105,25 @@ extern void appendStringInfo(StringInfo str, const char *fmt,...) pg_attribute_p
  */
 extern int	appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_attribute_printf(2, 0);
 
+/*------------------------
+ * appendBinaryStringInfo
+ * Append arbitrary binary data to a StringInfo, allocating more space
+ * if necessary.
+ */
+extern void appendBinaryStringInfo(StringInfo str, const char *data,
+								   int datalen);
+
 /*------------------------
  * appendStringInfoString
  * Append a null-terminated string to str.
- * Like appendStringInfo(str, "%s", s) but faster.
+ * Like appendStringInfo(str, "%s", s) but faster and inlined to allow
+ * compilers to optimize out the strlen call when used with string constants.
  */
-extern void appendStringInfoString(StringInfo str, const char *s);
+static inline void
+appendStringInfoString(StringInfo str, const char *s)
+{
+	appendBinaryStringInfo(str, s, strlen(s));
+}
 
 /*------------------------
  * appendStringInfoChar
@@ -135,14 +148,6 @@ extern void appendStringInfoChar(StringInfo str, char ch);
  */
 extern void appendStringInfoSpaces(StringInfo str, int count);
 
-/*------------------------
- * appendBinaryStringInfo
- * Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary.
- */
-extern void appendBinaryStringInfo(StringInfo str,
-								   const char *data, int datalen);
-
 /*------------------------
  * appendBinaryStringInfoNT
  * Append arbitrary binary data to a StringInfo, allocating more space
-- 
2.20.1