Patch to improve a few appendStringInfo* calls
Started by David Rowleyalmost 11 years ago12 messages
I've attached a small patch which just fixes a few appendStringInfo* calls
that are not quite doing things the way that it was intended.
Regards
David Rowley
Attachments:
appendstringinfo_fix.patchapplication/octet-stream; name=appendstringinfo_fix.patchDownload
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 478e124..bff6f45 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2720,7 +2720,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
appendStringInfoString(&buf, ", ");
deparseStringLiteral(&buf, rv->relname);
}
- appendStringInfoString(&buf, ")");
+ appendStringInfoChar(&buf, ')');
}
/* Append ORDER BY at the end of query to ensure output ordering */
@@ -2784,7 +2784,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
*/
appendStringInfoString(&buf, " OPTIONS (column_name ");
deparseStringLiteral(&buf, attname);
- appendStringInfoString(&buf, ")");
+ appendStringInfoChar(&buf, ')');
/* Add COLLATE if needed */
if (import_collate && collname != NULL && collnamespace != NULL)
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c
index 963d5df..7ec00df 100644
--- a/contrib/test_decoding/test_decoding.c
+++ b/contrib/test_decoding/test_decoding.c
@@ -391,7 +391,7 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
get_namespace_name(
get_rel_namespace(RelationGetRelid(relation))),
NameStr(class_form->relname)));
- appendStringInfoString(ctx->out, ":");
+ appendStringInfoChar(ctx->out, ':');
switch (change->action)
{
diff --git a/src/backend/access/rmgrdesc/smgrdesc.c b/src/backend/access/rmgrdesc/smgrdesc.c
index 4ae08b1..56d02c3 100644
--- a/src/backend/access/rmgrdesc/smgrdesc.c
+++ b/src/backend/access/rmgrdesc/smgrdesc.c
@@ -29,7 +29,7 @@ smgr_desc(StringInfo buf, XLogReaderState *record)
xl_smgr_create *xlrec = (xl_smgr_create *) rec;
char *path = relpathperm(xlrec->rnode, xlrec->forkNum);
- appendStringInfo(buf, "%s", path);
+ appendStringInfoString(buf, path);
pfree(path);
}
else if (info == XLOG_SMGR_TRUNCATE)
diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index 2c432f3..4f29136 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -75,7 +75,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
{
xl_restore_point *xlrec = (xl_restore_point *) rec;
- appendStringInfo(buf, "%s", xlrec->rp_name);
+ appendStringInfoString(buf, xlrec->rp_name);
}
else if (info == XLOG_FPI || info == XLOG_FPI_FOR_HINT)
{
@@ -125,7 +125,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
bool fpw;
memcpy(&fpw, rec, sizeof(bool));
- appendStringInfo(buf, "%s", fpw ? "true" : "false");
+ appendStringInfoString(buf, fpw ? "true" : "false");
}
else if (info == XLOG_END_OF_RECOVERY)
{
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 1af977c..2f1c8bf 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -2936,7 +2936,7 @@ NameListToString(List *names)
if (IsA(name, String))
appendStringInfoString(&string, strVal(name));
else if (IsA(name, A_Star))
- appendStringInfoString(&string, "*");
+ appendStringInfoChar(&string, '*');
else
elog(ERROR, "unexpected node type in name list: %d",
(int) nodeTag(name));
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 29b5b1b..0115fbf 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2176,7 +2176,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
appendStringInfoString(&rbuf, "TABLE(");
ntabargs = print_function_arguments(&rbuf, proctup, true, false);
if (ntabargs > 0)
- appendStringInfoString(&rbuf, ")");
+ appendStringInfoChar(&rbuf, ')');
else
resetStringInfo(&rbuf);
}
Re: Patch to improve a few appendStringInfo* calls
On 4/11/15 6:25 AM, David Rowley wrote:
I've attached a small patch which just fixes a few appendStringInfo*
calls that are not quite doing things the way that it was intended.
done
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
On 12 May 2015 at 12:57, Peter Eisentraut <peter_e@gmx.net> wrote:
On 4/11/15 6:25 AM, David Rowley wrote:
I've attached a small patch which just fixes a few appendStringInfo*
calls that are not quite doing things the way that it was intended.done
Thank you for pushing.
Shortly after I sent the previous patch I did a few more searches and also
found some more things that are not quite right.
Most of these are to use the binary append method when the length of the
string is already known.
There's also some fixes in there for appendPQExpBufferStr too.
I've re-based the patch and attached here.
Regards
David Rowley
Attachments:
appendStringInfo_fixes_2015-05-12.patchapplication/octet-stream; name=appendStringInfo_fixes_2015-05-12.patchDownload
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 7d89867..b405583 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1280,7 +1280,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&dst, '}');
- PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+ PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
}
PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1323,7 +1323,7 @@ hstore_to_json(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&dst, '}');
- PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+ PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
}
PG_FUNCTION_INFO_V1(hstore_to_jsonb);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0dee949..3bc06d9 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1969,7 +1969,7 @@ ExecBuildSlotValueDescription(Oid reloid,
if (!table_perm)
{
appendStringInfoString(&collist, ") = ");
- appendStringInfoString(&collist, buf.data);
+ appendBinaryStringInfo(&collist, buf.data, buf.len);
return collist.data;
}
diff --git a/src/backend/lib/pairingheap.c b/src/backend/lib/pairingheap.c
index 17278fd..53520eb 100644
--- a/src/backend/lib/pairingheap.c
+++ b/src/backend/lib/pairingheap.c
@@ -306,7 +306,7 @@ pairingheap_dump_recurse(StringInfo buf,
appendStringInfoSpaces(buf, depth * 4);
dumpfunc(node, buf, opaque);
- appendStringInfoString(buf, "\n");
+ appendStringInfoChar(buf, '\n');
if (node->first_child)
pairingheap_dump_recurse(buf, node->first_child, dumpfunc, opaque, depth + 1, node);
prev_or_parent = node;
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index aea46b2..e67e313 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -931,7 +931,7 @@ DeadLockReport(void)
}
/* Duplicate all the above for the server ... */
- appendStringInfoString(&logbuf, clientbuf.data);
+ appendBinaryStringInfo(&logbuf, clientbuf.data, clientbuf.len);
/* ... 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 f6bec8b..de18dc9 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -1316,7 +1316,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
queryoids[i] = pk_type;
queryoids[j] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys * 2, queryoids,
@@ -1481,7 +1481,7 @@ RI_FKey_setnull_del(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -1657,7 +1657,7 @@ RI_FKey_setnull_upd(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -1823,7 +1823,7 @@ RI_FKey_setdefault_del(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -2014,7 +2014,7 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* 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 903e80a..c8b4f6d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2067,9 +2067,9 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
appendStringInfoChar(&dq, 'x');
appendStringInfoChar(&dq, '$');
- appendStringInfoString(&buf, dq.data);
+ appendBinaryStringInfo(&buf, dq.data, dq.len);
appendStringInfoString(&buf, prosrc);
- appendStringInfoString(&buf, dq.data);
+ appendBinaryStringInfo(&buf, dq.data, dq.len);
appendStringInfoChar(&buf, '\n');
@@ -2187,7 +2187,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
appendStringInfoString(&rbuf, format_type_be(proc->prorettype));
}
- appendStringInfoString(buf, rbuf.data);
+ appendBinaryStringInfo(buf, rbuf.data, rbuf.len);
}
/*
@@ -4813,7 +4813,7 @@ get_target_list(List *targetList, deparse_context *context,
}
/* Add the new field */
- appendStringInfoString(buf, targetbuf.data);
+ appendBinaryStringInfo(buf, targetbuf.data, targetbuf.len);
}
/* clean up */
@@ -8494,7 +8494,7 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context)
}
/* Add the new item */
- appendStringInfoString(buf, itembuf.data);
+ appendBinaryStringInfo(buf, itembuf.data, itembuf.len);
/* clean up */
pfree(itembuf.data);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 8bb7144..f74d482 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -508,7 +508,7 @@ xmlconcat(List *args)
0,
global_standalone);
- appendStringInfoString(&buf2, buf.data);
+ appendBinaryStringInfo(&buf2, buf.data, buf.len);
buf = buf2;
}
@@ -1680,7 +1680,8 @@ xml_errorHandler(void *data, xmlErrorPtr error)
if (xmlerrcxt->strictness == PG_XML_STRICTNESS_LEGACY)
{
appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
- appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+ appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data,
+ errorBuf->len);
pfree(errorBuf->data);
pfree(errorBuf);
@@ -1698,7 +1699,8 @@ xml_errorHandler(void *data, xmlErrorPtr error)
if (level >= XML_ERR_ERROR)
{
appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
- appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+ appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data,
+ errorBuf->len);
xmlerrcxt->err_occurred = true;
}
@@ -2473,7 +2475,7 @@ query_to_xml_internal(const char *query, char *tablename,
{
xmldata_root_element_start(result, xmltn, xmlschema,
targetns, top_level);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
}
if (xmlschema)
@@ -2637,7 +2639,7 @@ schema_to_xml_internal(Oid nspid, const char *xmlschema, bool nulls,
result = makeStringInfo();
xmldata_root_element_start(result, xmlsn, xmlschema, targetns, top_level);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
if (xmlschema)
appendStringInfo(result, "%s\n\n", xmlschema);
@@ -2656,7 +2658,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);
+ appendBinaryStringInfo(result, subres->data, subres->len);
appendStringInfoChar(result, '\n');
}
@@ -2815,7 +2817,7 @@ database_to_xml_internal(const char *xmlschema, bool nulls,
result = makeStringInfo();
xmldata_root_element_start(result, xmlcn, xmlschema, targetns, true);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
if (xmlschema)
appendStringInfo(result, "%s\n\n", xmlschema);
@@ -2834,7 +2836,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);
+ appendBinaryStringInfo(result, subres->data, subres->len);
appendStringInfoChar(result, '\n');
}
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 83bf2f5..49d5987 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1516,7 +1516,7 @@ GenerateRecoveryConf(PGconn *conn)
/* Separate key-value pairs with spaces */
if (conninfo_buf.len != 0)
- appendPQExpBufferStr(&conninfo_buf, " ");
+ appendPQExpBufferChar(&conninfo_buf, ' ');
/*
* Write "keyword=value" pieces, the value string is escaped and/or
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..ec263c2 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -172,7 +172,7 @@ fmtQualifiedId(int remoteVersion, const char *schema, const char *id)
id_return = getLocalPQExpBuffer();
- appendPQExpBufferStr(id_return, lcl_pqexp->data);
+ appendBinaryPQExpBuffer(id_return, lcl_pqexp->data, lcl_pqexp->len);
destroyPQExpBuffer(lcl_pqexp);
return id_return->data;
@@ -331,9 +331,9 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix)
appendPQExpBufferChar(delimBuf, '$');
/* quote it and we are all done */
- appendPQExpBufferStr(buf, delimBuf->data);
+ appendBinaryPQExpBuffer(buf, delimBuf->data, delimBuf->len);
appendPQExpBufferStr(buf, str);
- appendPQExpBufferStr(buf, delimBuf->data);
+ appendBinaryPQExpBuffer(buf, delimBuf->data, delimBuf->len);
destroyPQExpBuffer(delimBuf);
}
@@ -1065,7 +1065,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
{
/* Found schema/name separator, move current pattern to schema */
resetPQExpBuffer(&schemabuf);
- appendPQExpBufferStr(&schemabuf, namebuf.data);
+ appendBinaryPQExpBuffer(&schemabuf, namebuf.data, namebuf.len);
resetPQExpBuffer(&namebuf);
appendPQExpBufferStr(&namebuf, "^(");
cp++;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index f9b564e..0d52bab 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -533,7 +533,7 @@ RestoreArchive(Archive *AHX)
* search for hardcoded "DROP CONSTRAINT" instead.
*/
if (strcmp(te->desc, "DEFAULT") == 0)
- appendPQExpBuffer(ftStmt, "%s", dropStmt);
+ appendPQExpBufferStr(ftStmt, dropStmt);
else
{
if (strcmp(te->desc, "CONSTRAINT") == 0 ||
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dccb472..b915b7b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1663,7 +1663,7 @@ dumpTableData_insert(Archive *fout, DumpOptions *dopt, void *dcontext)
/* append the list of column names if required */
if (dopt->column_inserts)
{
- appendPQExpBufferStr(insertStmt, "(");
+ appendPQExpBufferChar(insertStmt, '(');
for (field = 0; field < nfields; field++)
{
if (field > 0)
@@ -9550,7 +9550,7 @@ dumpCompositeType(Archive *fout, DumpOptions *dopt, TypeInfo *tyinfo)
}
}
appendPQExpBufferStr(q, "\n);\n");
- appendPQExpBufferStr(q, dropped->data);
+ appendBinaryPQExpBuffer(q, dropped->data, dropped->len);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
@@ -11338,7 +11338,7 @@ dumpOpclass(Archive *fout, DumpOptions *dopt, OpclassInfo *opcinfo)
appendPQExpBufferStr(q, " FAMILY ");
if (strcmp(opcfamilynsp, opcinfo->dobj.namespace->dobj.name) != 0)
appendPQExpBuffer(q, "%s.", fmtId(opcfamilynsp));
- appendPQExpBuffer(q, "%s", fmtId(opcfamilyname));
+ appendPQExpBufferStr(q, fmtId(opcfamilyname));
}
appendPQExpBufferStr(q, " AS\n ");
@@ -13850,7 +13850,7 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
if (actual_atts == 0)
appendPQExpBufferStr(q, " (");
else
- appendPQExpBufferStr(q, ",");
+ appendPQExpBufferChar(q, ',');
appendPQExpBufferStr(q, "\n ");
actual_atts++;
@@ -15283,7 +15283,7 @@ dumpEventTrigger(Archive *fout, DumpOptions *dopt, EventTriggerInfo *evtinfo)
appendPQExpBufferStr(query, fmtId(evtinfo->dobj.name));
appendPQExpBufferStr(query, " ON ");
appendPQExpBufferStr(query, fmtId(evtinfo->evtevent));
- appendPQExpBufferStr(query, " ");
+ appendPQExpBufferChar(query, ' ');
if (strcmp("", evtinfo->evttags) != 0)
{
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 04d769e..2c9b092 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1611,7 +1611,7 @@ describeOneTableDetails(const char *schemaname,
if (!PQgetisnull(res, i, 5))
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
appendPQExpBuffer(&tmpbuf, _("collate %s"),
PQgetvalue(res, i, 5));
}
@@ -1619,7 +1619,7 @@ describeOneTableDetails(const char *schemaname,
if (strcmp(PQgetvalue(res, i, 3), "t") == 0)
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
appendPQExpBufferStr(&tmpbuf, _("not null"));
}
@@ -1628,7 +1628,7 @@ describeOneTableDetails(const char *schemaname,
if (strlen(PQgetvalue(res, i, 2)) != 0)
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
/* translator: default values of column definitions */
appendPQExpBuffer(&tmpbuf, _("default %s"),
PQgetvalue(res, i, 2));
@@ -2440,7 +2440,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf, "%*s %s",
sw, "", PQgetvalue(result, i, 0));
if (i < tuples - 1)
- appendPQExpBufferStr(&buf, ",");
+ appendPQExpBufferChar(&buf, ',');
printTableAddFooter(&cont, buf.data);
}
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index b6cef94..f620957 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -345,7 +345,8 @@ MainLoop(FILE *source)
query_buf->len == 0)
{
/* copy previous buffer to current for handling */
- appendPQExpBufferStr(query_buf, previous_buf->data);
+ appendBinaryPQExpBuffer(query_buf, previous_buf->data,
+ previous_buf->len);
}
if (slashCmdStatus == PSQL_CMD_SEND)
diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 85087af..8c0e7cf 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -201,7 +201,7 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
appendPQExpBufferStr(&sql, " VERBOSE");
if (table)
appendPQExpBuffer(&sql, " %s", table);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false);
diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index a958bb8..4d3fb22 100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -195,7 +195,7 @@ main(int argc, char *argv[])
if (lc_ctype)
appendPQExpBuffer(&sql, " LC_CTYPE '%s'", lc_ctype);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
/* No point in trying to use postgres db when creating postgres db. */
if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
@@ -222,7 +222,7 @@ main(int argc, char *argv[])
{
printfPQExpBuffer(&sql, "COMMENT ON DATABASE %s IS ", fmtId(dbname));
appendStringLiteralConn(&sql, comment, conn);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index fba21a1..6fc898e 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -318,10 +318,10 @@ main(int argc, char *argv[])
if (cell->next)
appendPQExpBuffer(&sql, "%s,", fmtId(cell->val));
else
- appendPQExpBuffer(&sql, "%s", fmtId(cell->val));
+ appendPQExpBufferStr(&sql, fmtId(cell->val));
}
}
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index 778d72a..1f85b3e 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -281,7 +281,7 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
appendPQExpBuffer(&sql, " SCHEMA %s", name);
else if (strcmp(type, "DATABASE") == 0)
appendPQExpBuffer(&sql, " DATABASE %s", fmtId(name));
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false);
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 2cd4aa6..99e0a8a 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -392,7 +392,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
ntups = PQntuples(res);
for (i = 0; i < ntups; i++)
{
- appendPQExpBuffer(&buf, "%s",
+ appendPQExpBufferStr(&buf,
fmtQualifiedId(PQserverVersion(conn),
PQgetvalue(res, i, 1),
PQgetvalue(res, i, 0)));
@@ -643,7 +643,7 @@ prepare_vacuum_command(PQExpBuffer sql, PGconn *conn, vacuumingOptions *vacopts,
sep = comma;
}
if (sep != paren)
- appendPQExpBufferStr(sql, ")");
+ appendPQExpBufferChar(sql, ')');
}
else
{
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index a847f08..aa9409e 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -971,7 +971,8 @@ pqGetErrorNotice3(PGconn *conn, bool isError)
goto fail;
pqClearAsyncResult(conn);
conn->result = res;
- appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
+ appendBinaryPQExpBuffer(&conn->errorMessage, workBuf.data,
+ workBuf.len);
}
else
{
Re: Patch to improve a few appendStringInfo* calls
On 5/12/15 4:33 AM, David Rowley wrote:
Shortly after I sent the previous patch I did a few more searches and
also found some more things that are not quite right.
Most of these are to use the binary append method when the length of the
string is already known.
For these cases it might be better to invent additional functions such
as stringinfo_to_text() and appendStringInfoStringInfo() instead of
repeating the pattern of referring to data and length separately.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
On 29 May 2015 at 12:51, Peter Eisentraut <peter_e@gmx.net> wrote:
On 5/12/15 4:33 AM, David Rowley wrote:
Shortly after I sent the previous patch I did a few more searches and
also found some more things that are not quite right.
Most of these are to use the binary append method when the length of the
string is already known.For these cases it might be better to invent additional functions such
as stringinfo_to_text() and appendStringInfoStringInfo() instead of
repeating the pattern of referring to data and length separately.
You're probably right. It would be nicer to see some sort of wrapper
functions that cleaned these up a bit.
I really think that's something for another patch though, this patch just
intends to put things the way they're meant to be in the least invasive way
possible. What you're proposing is changing the way it's meant to work,
which will cause much more code churn.
I've attached a re-based patch.
Regards
David Rowley
--
David Rowley http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
appendStringInfo_fixes_fa48767_2015-06-15.patchapplication/octet-stream; name=appendStringInfo_fixes_fa48767_2015-06-15.patchDownload
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 7d89867..b405583 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1280,7 +1280,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&dst, '}');
- PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+ PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
}
PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1323,7 +1323,7 @@ hstore_to_json(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&dst, '}');
- PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+ PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
}
PG_FUNCTION_INFO_V1(hstore_to_jsonb);
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 6da01e1..e4d799c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2738,7 +2738,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
}
/* Append ORDER BY at the end of query to ensure output ordering */
- appendStringInfo(&buf, " ORDER BY c.relname, a.attnum");
+ appendStringInfoString(&buf, " ORDER BY c.relname, a.attnum");
/* Fetch the data */
res = PQexec(conn, buf.data);
diff --git a/src/backend/access/rmgrdesc/gindesc.c b/src/backend/access/rmgrdesc/gindesc.c
index 324efa3..09e928f 100644
--- a/src/backend/access/rmgrdesc/gindesc.c
+++ b/src/backend/access/rmgrdesc/gindesc.c
@@ -113,7 +113,7 @@ gin_desc(StringInfo buf, XLogReaderState *record)
(ginxlogRecompressDataLeaf *) payload;
if (XLogRecHasBlockImage(record, 0))
- appendStringInfo(buf, " (full page image)");
+ appendStringInfoString(buf, " (full page image)");
else
desc_recompress_leaf(buf, insertData);
}
@@ -147,7 +147,7 @@ gin_desc(StringInfo buf, XLogReaderState *record)
ginxlogVacuumDataLeafPage *xlrec = (ginxlogVacuumDataLeafPage *) rec;
if (XLogRecHasBlockImage(record, 0))
- appendStringInfo(buf, " (full page image)");
+ appendStringInfoString(buf, " (full page image)");
else
desc_recompress_leaf(buf, &xlrec->data);
}
diff --git a/src/backend/access/rmgrdesc/spgdesc.c b/src/backend/access/rmgrdesc/spgdesc.c
index 6e426d7..478f50c 100644
--- a/src/backend/access/rmgrdesc/spgdesc.c
+++ b/src/backend/access/rmgrdesc/spgdesc.c
@@ -30,14 +30,14 @@ spg_desc(StringInfo buf, XLogReaderState *record)
{
spgxlogAddLeaf *xlrec = (spgxlogAddLeaf *) rec;
- appendStringInfo(buf, "add leaf to page");
+ appendStringInfoString(buf, "add leaf to page");
appendStringInfo(buf, "; off %u; headoff %u; parentoff %u",
xlrec->offnumLeaf, xlrec->offnumHeadLeaf,
xlrec->offnumParent);
if (xlrec->newPage)
- appendStringInfo(buf, " (newpage)");
+ appendStringInfoString(buf, " (newpage)");
if (xlrec->storesNulls)
- appendStringInfo(buf, " (nulls)");
+ appendStringInfoString(buf, " (nulls)");
}
break;
case XLOG_SPGIST_MOVE_LEAFS:
@@ -63,9 +63,9 @@ spg_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, "ndel %u; nins %u",
xlrec->nDelete, xlrec->nInsert);
if (xlrec->innerIsParent)
- appendStringInfo(buf, " (innerIsParent)");
+ appendStringInfoString(buf, " (innerIsParent)");
if (xlrec->isRootSplit)
- appendStringInfo(buf, " (isRootSplit)");
+ appendStringInfoString(buf, " (isRootSplit)");
}
break;
case XLOG_SPGIST_VACUUM_LEAF:
diff --git a/src/backend/access/rmgrdesc/xactdesc.c b/src/backend/access/rmgrdesc/xactdesc.c
index 7b5f983..e811c0a 100644
--- a/src/backend/access/rmgrdesc/xactdesc.c
+++ b/src/backend/access/rmgrdesc/xactdesc.c
@@ -232,7 +232,7 @@ xact_desc_commit(StringInfo buf, uint8 info, xl_xact_commit *xlrec, RepOriginId
}
if (XactCompletionForceSyncCommit(parsed.xinfo))
- appendStringInfo(buf, "; sync");
+ appendStringInfoString(buf, "; sync");
if (parsed.xinfo & XACT_XINFO_HAS_ORIGIN)
{
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 150d56a..43d540a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1096,7 +1096,7 @@ XLogInsertRecord(XLogRecData *rdata, XLogRecPtr fpw_lsn)
if (!debug_reader)
{
- appendStringInfo(&buf, "error decoding record: out of memory");
+ appendStringInfoString(&buf, "error decoding record: out of memory");
}
else if (!DecodeXLogRecord(debug_reader, (XLogRecord *) recordBuf.data,
&errormsg))
@@ -9542,7 +9542,7 @@ xlog_outrec(StringInfo buf, XLogReaderState *record)
rnode.spcNode, rnode.dbNode, rnode.relNode,
blk);
if (XLogRecHasBlockImage(record, block_id))
- appendStringInfo(buf, " FPW");
+ appendStringInfoString(buf, " FPW");
}
}
#endif /* WAL_DEBUG */
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index a1561ce..403423d 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1976,7 +1976,7 @@ ExecBuildSlotValueDescription(Oid reloid,
if (!table_perm)
{
appendStringInfoString(&collist, ") = ");
- appendStringInfoString(&collist, buf.data);
+ appendBinaryStringInfo(&collist, buf.data, buf.len);
return collist.data;
}
diff --git a/src/backend/lib/pairingheap.c b/src/backend/lib/pairingheap.c
index 3d8a5ea..7ca3545 100644
--- a/src/backend/lib/pairingheap.c
+++ b/src/backend/lib/pairingheap.c
@@ -306,7 +306,7 @@ pairingheap_dump_recurse(StringInfo buf,
appendStringInfoSpaces(buf, depth * 4);
dumpfunc(node, buf, opaque);
- appendStringInfoString(buf, "\n");
+ appendStringInfoChar(buf, '\n');
if (node->first_child)
pairingheap_dump_recurse(buf, node->first_child, dumpfunc, opaque, depth + 1, node);
prev_or_parent = node;
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index aea46b2..e67e313 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -931,7 +931,7 @@ DeadLockReport(void)
}
/* Duplicate all the above for the server ... */
- appendStringInfoString(&logbuf, clientbuf.data);
+ appendBinaryStringInfo(&logbuf, clientbuf.data, clientbuf.len);
/* ... 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 88dd3fa..6c584db 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -1316,7 +1316,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
queryoids[i] = pk_type;
queryoids[j] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys * 2, queryoids,
@@ -1481,7 +1481,7 @@ RI_FKey_setnull_del(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -1657,7 +1657,7 @@ RI_FKey_setnull_upd(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -1823,7 +1823,7 @@ RI_FKey_setdefault_del(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* Prepare and save the plan */
qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
@@ -2014,7 +2014,7 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS)
qualsep = "AND";
queryoids[i] = pk_type;
}
- appendStringInfoString(&querybuf, qualbuf.data);
+ appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
/* 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 e316951..2df4f12 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2079,9 +2079,9 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
appendStringInfoChar(&dq, 'x');
appendStringInfoChar(&dq, '$');
- appendStringInfoString(&buf, dq.data);
+ appendBinaryStringInfo(&buf, dq.data, dq.len);
appendStringInfoString(&buf, prosrc);
- appendStringInfoString(&buf, dq.data);
+ appendBinaryStringInfo(&buf, dq.data, dq.len);
appendStringInfoChar(&buf, '\n');
@@ -2199,7 +2199,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
appendStringInfoString(&rbuf, format_type_be(proc->prorettype));
}
- appendStringInfoString(buf, rbuf.data);
+ appendBinaryStringInfo(buf, rbuf.data, rbuf.len);
}
/*
@@ -4895,7 +4895,7 @@ get_target_list(List *targetList, deparse_context *context,
}
/* Add the new field */
- appendStringInfoString(buf, targetbuf.data);
+ appendBinaryStringInfo(buf, targetbuf.data, targetbuf.len);
}
/* clean up */
@@ -5486,7 +5486,7 @@ get_insert_query_def(Query *query, deparse_context *context)
{
OnConflictExpr *confl = query->onConflict;
- appendStringInfo(buf, " ON CONFLICT");
+ appendStringInfoString(buf, " ON CONFLICT");
if (confl->arbiterElems)
{
@@ -8745,7 +8745,7 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context)
}
/* Add the new item */
- appendStringInfoString(buf, itembuf.data);
+ appendBinaryStringInfo(buf, itembuf.data, itembuf.len);
/* clean up */
pfree(itembuf.data);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 99bc832..600bd18 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -508,7 +508,7 @@ xmlconcat(List *args)
0,
global_standalone);
- appendStringInfoString(&buf2, buf.data);
+ appendBinaryStringInfo(&buf2, buf.data, buf.len);
buf = buf2;
}
@@ -1680,7 +1680,8 @@ xml_errorHandler(void *data, xmlErrorPtr error)
if (xmlerrcxt->strictness == PG_XML_STRICTNESS_LEGACY)
{
appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
- appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+ appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data,
+ errorBuf->len);
pfree(errorBuf->data);
pfree(errorBuf);
@@ -1698,7 +1699,8 @@ xml_errorHandler(void *data, xmlErrorPtr error)
if (level >= XML_ERR_ERROR)
{
appendStringInfoLineSeparator(&xmlerrcxt->err_buf);
- appendStringInfoString(&xmlerrcxt->err_buf, errorBuf->data);
+ appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data,
+ errorBuf->len);
xmlerrcxt->err_occurred = true;
}
@@ -2473,7 +2475,7 @@ query_to_xml_internal(const char *query, char *tablename,
{
xmldata_root_element_start(result, xmltn, xmlschema,
targetns, top_level);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
}
if (xmlschema)
@@ -2637,7 +2639,7 @@ schema_to_xml_internal(Oid nspid, const char *xmlschema, bool nulls,
result = makeStringInfo();
xmldata_root_element_start(result, xmlsn, xmlschema, targetns, top_level);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
if (xmlschema)
appendStringInfo(result, "%s\n\n", xmlschema);
@@ -2656,7 +2658,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);
+ appendBinaryStringInfo(result, subres->data, subres->len);
appendStringInfoChar(result, '\n');
}
@@ -2815,7 +2817,7 @@ database_to_xml_internal(const char *xmlschema, bool nulls,
result = makeStringInfo();
xmldata_root_element_start(result, xmlcn, xmlschema, targetns, true);
- appendStringInfoString(result, "\n");
+ appendStringInfoChar(result, '\n');
if (xmlschema)
appendStringInfo(result, "%s\n\n", xmlschema);
@@ -2834,7 +2836,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);
+ appendBinaryStringInfo(result, subres->data, subres->len);
appendStringInfoChar(result, '\n');
}
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 5dd2887..5363680 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1516,7 +1516,7 @@ GenerateRecoveryConf(PGconn *conn)
/* Separate key-value pairs with spaces */
if (conninfo_buf.len != 0)
- appendPQExpBufferStr(&conninfo_buf, " ");
+ appendPQExpBufferChar(&conninfo_buf, ' ');
/*
* Write "keyword=value" pieces, the value string is escaped and/or
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..ec263c2 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -172,7 +172,7 @@ fmtQualifiedId(int remoteVersion, const char *schema, const char *id)
id_return = getLocalPQExpBuffer();
- appendPQExpBufferStr(id_return, lcl_pqexp->data);
+ appendBinaryPQExpBuffer(id_return, lcl_pqexp->data, lcl_pqexp->len);
destroyPQExpBuffer(lcl_pqexp);
return id_return->data;
@@ -331,9 +331,9 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix)
appendPQExpBufferChar(delimBuf, '$');
/* quote it and we are all done */
- appendPQExpBufferStr(buf, delimBuf->data);
+ appendBinaryPQExpBuffer(buf, delimBuf->data, delimBuf->len);
appendPQExpBufferStr(buf, str);
- appendPQExpBufferStr(buf, delimBuf->data);
+ appendBinaryPQExpBuffer(buf, delimBuf->data, delimBuf->len);
destroyPQExpBuffer(delimBuf);
}
@@ -1065,7 +1065,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
{
/* Found schema/name separator, move current pattern to schema */
resetPQExpBuffer(&schemabuf);
- appendPQExpBufferStr(&schemabuf, namebuf.data);
+ appendBinaryPQExpBuffer(&schemabuf, namebuf.data, namebuf.len);
resetPQExpBuffer(&namebuf);
appendPQExpBufferStr(&namebuf, "^(");
cp++;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index f9b564e..0d52bab 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -533,7 +533,7 @@ RestoreArchive(Archive *AHX)
* search for hardcoded "DROP CONSTRAINT" instead.
*/
if (strcmp(te->desc, "DEFAULT") == 0)
- appendPQExpBuffer(ftStmt, "%s", dropStmt);
+ appendPQExpBufferStr(ftStmt, dropStmt);
else
{
if (strcmp(te->desc, "CONSTRAINT") == 0 ||
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a72dfe9..6cbacd2 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1659,7 +1659,7 @@ dumpTableData_insert(Archive *fout, DumpOptions *dopt, void *dcontext)
/* append the list of column names if required */
if (dopt->column_inserts)
{
- appendPQExpBufferStr(insertStmt, "(");
+ appendPQExpBufferChar(insertStmt, '(');
for (field = 0; field < nfields; field++)
{
if (field > 0)
@@ -9546,7 +9546,7 @@ dumpCompositeType(Archive *fout, DumpOptions *dopt, TypeInfo *tyinfo)
}
}
appendPQExpBufferStr(q, "\n);\n");
- appendPQExpBufferStr(q, dropped->data);
+ appendBinaryPQExpBuffer(q, dropped->data, dropped->len);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
@@ -11332,7 +11332,7 @@ dumpOpclass(Archive *fout, DumpOptions *dopt, OpclassInfo *opcinfo)
appendPQExpBufferStr(q, " FAMILY ");
if (strcmp(opcfamilynsp, opcinfo->dobj.namespace->dobj.name) != 0)
appendPQExpBuffer(q, "%s.", fmtId(opcfamilynsp));
- appendPQExpBuffer(q, "%s", fmtId(opcfamilyname));
+ appendPQExpBufferStr(q, fmtId(opcfamilyname));
}
appendPQExpBufferStr(q, " AS\n ");
@@ -13844,7 +13844,7 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
if (actual_atts == 0)
appendPQExpBufferStr(q, " (");
else
- appendPQExpBufferStr(q, ",");
+ appendPQExpBufferChar(q, ',');
appendPQExpBufferStr(q, "\n ");
actual_atts++;
@@ -15277,7 +15277,7 @@ dumpEventTrigger(Archive *fout, DumpOptions *dopt, EventTriggerInfo *evtinfo)
appendPQExpBufferStr(query, fmtId(evtinfo->dobj.name));
appendPQExpBufferStr(query, " ON ");
appendPQExpBufferStr(query, fmtId(evtinfo->evtevent));
- appendPQExpBufferStr(query, " ");
+ appendPQExpBufferChar(query, ' ');
if (strcmp("", evtinfo->evttags) != 0)
{
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index db56809..f63c7e9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1611,7 +1611,7 @@ describeOneTableDetails(const char *schemaname,
if (!PQgetisnull(res, i, 5))
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
appendPQExpBuffer(&tmpbuf, _("collate %s"),
PQgetvalue(res, i, 5));
}
@@ -1619,7 +1619,7 @@ describeOneTableDetails(const char *schemaname,
if (strcmp(PQgetvalue(res, i, 3), "t") == 0)
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
appendPQExpBufferStr(&tmpbuf, _("not null"));
}
@@ -1628,7 +1628,7 @@ describeOneTableDetails(const char *schemaname,
if (strlen(PQgetvalue(res, i, 2)) != 0)
{
if (tmpbuf.len > 0)
- appendPQExpBufferStr(&tmpbuf, " ");
+ appendPQExpBufferChar(&tmpbuf, ' ');
/* translator: default values of column definitions */
appendPQExpBuffer(&tmpbuf, _("default %s"),
PQgetvalue(res, i, 2));
@@ -2440,7 +2440,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf, "%*s %s",
sw, "", PQgetvalue(result, i, 0));
if (i < tuples - 1)
- appendPQExpBufferStr(&buf, ",");
+ appendPQExpBufferChar(&buf, ',');
printTableAddFooter(&cont, buf.data);
}
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index b6cef94..f620957 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -345,7 +345,8 @@ MainLoop(FILE *source)
query_buf->len == 0)
{
/* copy previous buffer to current for handling */
- appendPQExpBufferStr(query_buf, previous_buf->data);
+ appendBinaryPQExpBuffer(query_buf, previous_buf->data,
+ previous_buf->len);
}
if (slashCmdStatus == PSQL_CMD_SEND)
diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 85087af..8c0e7cf 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -201,7 +201,7 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
appendPQExpBufferStr(&sql, " VERBOSE");
if (table)
appendPQExpBuffer(&sql, " %s", table);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false);
diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index a958bb8..4d3fb22 100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -195,7 +195,7 @@ main(int argc, char *argv[])
if (lc_ctype)
appendPQExpBuffer(&sql, " LC_CTYPE '%s'", lc_ctype);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
/* No point in trying to use postgres db when creating postgres db. */
if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
@@ -222,7 +222,7 @@ main(int argc, char *argv[])
{
printfPQExpBuffer(&sql, "COMMENT ON DATABASE %s IS ", fmtId(dbname));
appendStringLiteralConn(&sql, comment, conn);
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index fba21a1..6fc898e 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -318,10 +318,10 @@ main(int argc, char *argv[])
if (cell->next)
appendPQExpBuffer(&sql, "%s,", fmtId(cell->val));
else
- appendPQExpBuffer(&sql, "%s", fmtId(cell->val));
+ appendPQExpBufferStr(&sql, fmtId(cell->val));
}
}
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index 941729d..80c7886 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -295,7 +295,7 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
appendPQExpBuffer(&sql, " SCHEMA %s", name);
else if (strcmp(type, "DATABASE") == 0)
appendPQExpBuffer(&sql, " DATABASE %s", fmtId(name));
- appendPQExpBufferStr(&sql, ";");
+ appendPQExpBufferChar(&sql, ';');
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false);
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index f600b05..ca6d003 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -392,7 +392,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
ntups = PQntuples(res);
for (i = 0; i < ntups; i++)
{
- appendPQExpBuffer(&buf, "%s",
+ appendPQExpBufferStr(&buf,
fmtQualifiedId(PQserverVersion(conn),
PQgetvalue(res, i, 1),
PQgetvalue(res, i, 0)));
@@ -643,7 +643,7 @@ prepare_vacuum_command(PQExpBuffer sql, PGconn *conn, vacuumingOptions *vacopts,
sep = comma;
}
if (sep != paren)
- appendPQExpBufferStr(sql, ")");
+ appendPQExpBufferChar(sql, ')');
}
else
{
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index a847f08..aa9409e 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -971,7 +971,8 @@ pqGetErrorNotice3(PGconn *conn, bool isError)
goto fail;
pqClearAsyncResult(conn);
conn->result = res;
- appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
+ appendBinaryPQExpBuffer(&conn->errorMessage, workBuf.data,
+ workBuf.len);
}
else
{
Re: Patch to improve a few appendStringInfo* calls
On 06/15/2015 03:56 AM, David Rowley wrote:
On 29 May 2015 at 12:51, Peter Eisentraut <peter_e@gmx.net> wrote:
On 5/12/15 4:33 AM, David Rowley wrote:
Shortly after I sent the previous patch I did a few more searches and
also found some more things that are not quite right.
Most of these are to use the binary append method when the length of the
string is already known.For these cases it might be better to invent additional functions such
as stringinfo_to_text() and appendStringInfoStringInfo() instead of
repeating the pattern of referring to data and length separately.You're probably right. It would be nicer to see some sort of wrapper
functions that cleaned these up a bit.I really think that's something for another patch though, this patch just
intends to put things the way they're meant to be in the least invasive way
possible. What you're proposing is changing the way it's meant to work,
which will cause much more code churn.I've attached a re-based patch.
Applied the straightforward parts. I left out the changes like
- appendStringInfoString(&collist, buf.data); + appendBinaryStringInfo(&collist, buf.data, buf.len);
because they're not an improvement in readablity, IMHO, and they were
not in performance-critical paths.
I also noticed that the space after "CREATE EVENT TRIGGER <name>" in
pg_dump was actually spurious, and just added a space before newline. I
removed that space altogether,
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
On 2 July 2015 at 21:59, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
Applied the straightforward parts.
Thanks for committing.
I left out the changes like
- appendStringInfoString(&collist, buf.data);
+ appendBinaryStringInfo(&collist, buf.data, buf.len);
because they're not an improvement in readablity, IMHO, and they were not
in performance-critical paths.
Perhaps we can come up with appendStringInfoStringInfo at some later date.
--
David Rowley http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Training & Services
Re: Patch to improve a few appendStringInfo* calls
David Rowley wrote:
On 2 July 2015 at 21:59, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
I left out the changes like
- appendStringInfoString(&collist, buf.data);
+ appendBinaryStringInfo(&collist, buf.data, buf.len);
because they're not an improvement in readablity, IMHO, and they were not
in performance-critical paths.Perhaps we can come up with appendStringInfoStringInfo at some later date.
I had this exact thought when I saw your patch (though it was
appendStringInfoSI to me because the other name is too long and a bit
confusing). It seems straightforward enough ...
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
On Thu, Jul 2, 2015 at 6:08 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
I left out the changes like
- appendStringInfoString(&collist, buf.data); + appendBinaryStringInfo(&collist, buf.data, buf.len);because they're not an improvement in readablity, IMHO, and they were not
in performance-critical paths.Perhaps we can come up with appendStringInfoStringInfo at some later date.
concatenateStringInfo?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
2015-12-21 13:58 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Thu, Jul 2, 2015 at 6:08 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:I left out the changes like
- appendStringInfoString(&collist, buf.data); + appendBinaryStringInfo(&collist, buf.data, buf.len);because they're not an improvement in readablity, IMHO, and they were
not
in performance-critical paths.
Perhaps we can come up with appendStringInfoStringInfo at some later
date.
concatenateStringInfo?
concatStringInfo ?
Pavel
Show quoted text
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Patch to improve a few appendStringInfo* calls
On 22 December 2015 at 01:58, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Jul 2, 2015 at 6:08 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:I left out the changes like
- appendStringInfoString(&collist, buf.data); + appendBinaryStringInfo(&collist, buf.data, buf.len);because they're not an improvement in readablity, IMHO, and they were
not
in performance-critical paths.
Perhaps we can come up with appendStringInfoStringInfo at some later
date.
concatenateStringInfo?
According to grep -rE "appendStringInfoString\(.*\.data\);" . we have 13
such matches. None of them seem to be in very performance critical places,
perhaps with the exception of xmlconcat().
Would you say that we should build a macro to wrap up a call
to appendBinaryStringInfo() or invent another function which looks similar?
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Re: Patch to improve a few appendStringInfo* calls
On Mon, Dec 21, 2015 at 6:28 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
According to grep -rE "appendStringInfoString\(.*\.data\);" . we have 13
such matches. None of them seem to be in very performance critical places,
perhaps with the exception of xmlconcat().Would you say that we should build a macro to wrap up a call to
appendBinaryStringInfo() or invent another function which looks similar?
The macro probably would have a multiple evaluation hazard, so I'd be
inclined to invent a function. I mean, yeah, you could use a do { }
while (0) block to fix the multiple evaluation, but complex macros are
harder to read than functions, and not necessarily any faster.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers