ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > David Rowley <david.row...@2ndquadrant.com> writes: > >> On Thu, 4 Jul 2019 at 13:51, David Rowley <david.row...@2ndquadrant.com> >> wrote: >>> Instead of having 0004, how about the attached? >>> >>> Most of the calls won't improve much performance-wise since they're so >>> cheap anyway, but there is xmlconcat(), I imagine that should see some >>> speedup. >> >> I've pushed this after having found a couple more places where the >> length is known. > > I noticed a lot of these are appending one StringInfo onto another; > would it make sense to introduce a helper funciton > appendStringInfoStringInfo(StringInfo str, StringInfo str2) to avoid the > `str.data, str2.len` repetition?
A bit of grepping only turned up 18 uses, but I was bored and whipped up the attached anyway, in case we decide it's worth it. - ilmari -- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law
>From 1e68adae513425470cad10cd2a44f66bca61a5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org> Date: Mon, 22 Jul 2019 15:01:03 +0100 Subject: [PATCH] Add appendStringInfoStringInfo() function This simplifies appending one StringInfo to another --- contrib/hstore/hstore_io.c | 2 +- contrib/postgres_fdw/deparse.c | 4 ++-- src/backend/access/transam/xlog.c | 2 +- src/backend/executor/execMain.c | 2 +- src/backend/lib/stringinfo.c | 12 ++++++++++++ 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 | 12 +++++------- src/include/lib/stringinfo.h | 7 +++++++ 10 files changed, 37 insertions(+), 20 deletions(-) diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c index 745497c76f..58415ccaec 100644 --- a/contrib/hstore/hstore_io.c +++ b/contrib/hstore/hstore_io.c @@ -1309,7 +1309,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS) appendBinaryStringInfo(&tmp, HSTORE_VAL(entries, base, i), HSTORE_VALLEN(entries, i)); if (IsValidJsonNumber(tmp.data, tmp.len)) - appendBinaryStringInfo(&dst, tmp.data, tmp.len); + appendStringInfoStringInfo(&dst, &tmp); else escape_json(&dst, tmp.data); } diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 19f928ec59..510e034e8e 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); - appendBinaryStringInfo(buf, join_sql_o.data, join_sql_o.len); + 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); - appendBinaryStringInfo(buf, join_sql_i.data, join_sql_i.len); + appendStringInfoStringInfo(buf, &join_sql_i); return; } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index da3d250986..e674c63056 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1192,7 +1192,7 @@ XLogInsertRecord(XLogRecData *rdata, */ initStringInfo(&recordBuf); for (; rdata != NULL; rdata = rdata->next) - appendBinaryStringInfo(&recordBuf, rdata->data, rdata->len); + appendStringInfoStringInfo(&recordBuf, rdata); if (!debug_reader) debug_reader = XLogReaderAllocate(wal_segment_size, NULL, NULL); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index dbd7dd9bcd..1969b36891 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, ") = "); - appendBinaryStringInfo(&collist, buf.data, buf.len); + appendStringInfoStringInfo(&collist, &buf); return collist.data; } diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c index 99c83c1549..d25d122289 100644 --- a/src/backend/lib/stringinfo.c +++ b/src/backend/lib/stringinfo.c @@ -165,6 +165,18 @@ appendStringInfoString(StringInfo str, const char *s) appendBinaryStringInfo(str, s, strlen(s)); } +/* + * appendStringInfoStringInfo + * + * Append another StringInfo to str. + * Like appendBinaryStringInfo(str, str2->data, str2->len) + */ +void +appendStringInfoStringInfo(StringInfo str, const StringInfo str2) +{ + appendBinaryStringInfo(str, str2->data, str2->len); +} + /* * appendStringInfoChar * diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index 990d48377d..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 ... */ - appendBinaryStringInfo(&logbuf, clientbuf.data, clientbuf.len); + 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 8c895459c3..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; } - appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len); + 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; } - appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len); + 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 0c58f1f109..1801899ec1 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, '$'); - appendBinaryStringInfo(&buf, dq.data, dq.len); + appendStringInfoStringInfo(&buf, &dq); appendStringInfoString(&buf, prosrc); - appendBinaryStringInfo(&buf, dq.data, dq.len); + appendStringInfoStringInfo(&buf, &dq); appendStringInfoChar(&buf, '\n'); @@ -2930,7 +2930,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup) appendStringInfoString(&rbuf, format_type_be(proc->prorettype)); } - appendBinaryStringInfo(buf, rbuf.data, rbuf.len); + appendStringInfoStringInfo(buf, &rbuf); } /* @@ -5682,7 +5682,7 @@ get_target_list(List *targetList, deparse_context *context, } /* Add the new field */ - appendBinaryStringInfo(buf, targetbuf.data, targetbuf.len); + appendStringInfoStringInfo(buf, &targetbuf); } /* clean up */ @@ -9987,7 +9987,7 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context) } /* Add the new item */ - appendBinaryStringInfo(buf, itembuf.data, itembuf.len); + 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 5e629d29ea..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); - appendBinaryStringInfo(&buf2, buf.data, buf.len); + appendStringInfoStringInfo(&buf2, &buf); buf = buf2; } @@ -1879,8 +1879,7 @@ xml_errorHandler(void *data, xmlErrorPtr error) if (xmlerrcxt->strictness == PG_XML_STRICTNESS_LEGACY) { appendStringInfoLineSeparator(&xmlerrcxt->err_buf); - appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data, - errorBuf->len); + appendStringInfoStringInfo(&xmlerrcxt->err_buf, errorBuf); pfree(errorBuf->data); pfree(errorBuf); @@ -1898,8 +1897,7 @@ xml_errorHandler(void *data, xmlErrorPtr error) if (level >= XML_ERR_ERROR) { appendStringInfoLineSeparator(&xmlerrcxt->err_buf); - appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data, - errorBuf->len); + appendStringInfoStringInfo(&xmlerrcxt->err_buf, errorBuf); xmlerrcxt->err_occurred = true; } @@ -2876,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); - appendBinaryStringInfo(result, subres->data, subres->len); + appendStringInfoStringInfo(result, subres); appendStringInfoChar(result, '\n'); } @@ -3051,7 +3049,7 @@ database_to_xml_internal(const char *xmlschema, bool nulls, subres = schema_to_xml_internal(nspid, NULL, nulls, tableforest, targetns, false); - appendBinaryStringInfo(result, subres->data, subres->len); + appendStringInfoStringInfo(result, subres); appendStringInfoChar(result, '\n'); } diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index c4842778c5..8a6ffd7ce8 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -112,6 +112,13 @@ extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_ */ extern void appendStringInfoString(StringInfo str, const char *s); +/*------------------------ + * appendStringInfoString + * Append another StringInfo to str. + * Like appendBinaryStringInfo(str, str2->data, str2->len) + */ +extern void appendStringInfoStringInfo(StringInfo str, const StringInfo str2); + /*------------------------ * appendStringInfoChar * Append a single byte to str. -- 2.20.1