On Mon, 5 Sept 2022 at 22:15, David Rowley <dgrowle...@gmail.com> wrote:
> On Sat, 3 Sept 2022 at 00:37, Ranier Vilela <ranier...@gmail.com> wrote:
> > 6. Avoid overhead when using unnecessary StringInfoData to convert Datum a
> > to Text b.
>
> I've ripped out #4 and #6 for now. I think we should do #6 in master
> only, probably as part of a wider cleanup of StringInfo misusages.
I've attached a patch which does various other string operation cleanups.
* This changes cstring_to_text() to use cstring_to_text_with_len when
we're working with a StringInfo and can just access the .len field.
* Uses appendStringInfoString instead of appendStringInfo when there
is special formatting.
* Uses pstrdup(str) instead of psprintf("%s", str). In many cases
this will save a bit of memory
* Uses appendPQExpBufferChar instead of appendPQExpBufferStr() when
appending a 1 byte string.
* Uses appendStringInfoChar() instead of appendStringInfo() when no
formatting and string is 1 byte.
* Uses appendStringInfoChar() instead of appendStringInfoString() when
string is 1 byte.
* Uses appendPQExpBuffer(b , ...) instead of appendPQExpBufferStr(b, "%s" ...)
I'm aware there are a few other places that we could use
cstring_to_text_with_len() instead of cstring_to_text(). For example,
using the return value of snprintf() to obtain the length. I just
didn't do that because we need to take care to check the return value
isn't -1.
My grep patterns didn't account for these function calls spanning
multiple lines, so I may have missed a few.
David
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index fb72bb6cfe..6161df2790 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1325,7 +1325,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);
@@ -1370,7 +1370,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/access/brin/brin_minmax_multi.c
b/src/backend/access/brin/brin_minmax_multi.c
index a581659fe2..ea326a250b 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -3063,7 +3063,7 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
appendStringInfo(&str, "%s ... %s", a, b);
- c = cstring_to_text(str.data);
+ c = cstring_to_text_with_len(str.data, str.len);
astate_values = accumArrayResult(astate_values,
PointerGetDatum(c),
@@ -3095,15 +3095,9 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
{
Datum a;
text *b;
- StringInfoData str;
-
- initStringInfo(&str);
a = FunctionCall1(&fmgrinfo,
ranges_deserialized->values[idx++]);
-
- appendStringInfoString(&str, DatumGetCString(a));
-
- b = cstring_to_text(str.data);
+ b = cstring_to_text(DatumGetCString(a));
astate_values = accumArrayResult(astate_values,
PointerGetDatum(b),
diff --git a/src/backend/replication/logical/tablesync.c
b/src/backend/replication/logical/tablesync.c
index 91ba49a14b..78abbb8196 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -1062,7 +1062,7 @@ copy_table(Relation rel)
appendStringInfoString(&cmd,
quote_identifier(lrel.attnames[i]));
}
- appendStringInfo(&cmd, ") TO STDOUT");
+ appendStringInfoString(&cmd, ") TO STDOUT");
}
else
{
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 081dfa2450..a2bdde0459 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -96,7 +96,7 @@ anytime_typmodout(bool istz, int32 typmod)
if (typmod >= 0)
return psprintf("(%d)%s", (int) typmod, tz);
else
- return psprintf("%s", tz);
+ return pstrdup(tz);
}
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 6594a9aac7..2b7b1b0c0f 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1453,7 +1453,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
appendStringInfoChar(&buf, ')');
if (idxrec->indnullsnotdistinct)
- appendStringInfo(&buf, " NULLS NOT DISTINCT");
+ appendStringInfoString(&buf, " NULLS NOT DISTINCT");
/*
* If it has options, append "WITH (options)"
@@ -2332,9 +2332,9 @@ pg_get_constraintdef_worker(Oid constraintId, bool
fullCommand,
Anum_pg_constraint_confdelsetcols, &isnull);
if (!isnull)
{
- appendStringInfo(&buf, " (");
+ appendStringInfoString(&buf, " (");
decompile_column_index_array(val,
conForm->conrelid, &buf);
- appendStringInfo(&buf, ")");
+ appendStringInfoChar(&buf, ')');
}
break;
@@ -2363,7 +2363,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool
fullCommand,
((Form_pg_index)
GETSTRUCT(indtup))->indnullsnotdistinct)
appendStringInfoString(&buf, "NULLS NOT
DISTINCT ");
- appendStringInfoString(&buf, "(");
+ appendStringInfoChar(&buf, '(');
/* Fetch and build target column list */
val = SysCacheGetAttr(CONSTROID, tup,
@@ -3583,7 +3583,7 @@ pg_get_function_sqlbody(PG_FUNCTION_ARGS)
ReleaseSysCache(proctup);
- PG_RETURN_TEXT_P(cstring_to_text(buf.data));
+ PG_RETURN_TEXT_P(cstring_to_text_with_len(buf.data, buf.len));
}
diff --git a/src/backend/utils/adt/timestamp.c
b/src/backend/utils/adt/timestamp.c
index 49cdb290ac..021b760f4e 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -131,7 +131,7 @@ anytimestamp_typmodout(bool istz, int32 typmod)
if (typmod >= 0)
return psprintf("(%d)%s", (int) typmod, tz);
else
- return psprintf("%s", tz);
+ return pstrdup(tz);
}
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 3cff319f02..0356e132db 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -1509,7 +1509,7 @@ append_db_pattern_cte(PQExpBuffer buf, const
PatternInfoArray *pia,
have_values = true;
appendPQExpBuffer(buf, "%s\n(%d, ", comma, pattern_id);
appendStringLiteralConn(buf, info->db_regex, conn);
- appendPQExpBufferStr(buf, ")");
+ appendPQExpBufferChar(buf, ')');
comma = ",";
}
}
@@ -1765,7 +1765,7 @@ append_rel_pattern_raw_cte(PQExpBuffer buf, const
PatternInfoArray *pia,
appendPQExpBufferStr(buf, ", true::BOOLEAN");
else
appendPQExpBufferStr(buf, ", false::BOOLEAN");
- appendPQExpBufferStr(buf, ")");
+ appendPQExpBufferChar(buf, ')');
comma = ",";
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d25709ad5f..f4c8b028f9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4045,7 +4045,7 @@ dumpPublication(Archive *fout, const PublicationInfo
*pubinfo)
first = false;
}
- appendPQExpBufferStr(query, "'");
+ appendPQExpBufferChar(query, '\'');
if (pubinfo->pubviaroot)
appendPQExpBufferStr(query, ", publish_via_partition_root =
true");
@@ -15491,7 +15491,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
appendStringLiteralAH(q, qualrelname,
fout);
appendPQExpBufferStr(q,
"::pg_catalog.regclass,");
appendStringLiteralAH(q,
tbinfo->attnames[j], fout);
- appendPQExpBufferStr(q, ",");
+ appendPQExpBufferChar(q, ',');
appendStringLiteralAH(q,
tbinfo->attmissingval[j], fout);
appendPQExpBufferStr(q, ");\n\n");
}
@@ -16361,8 +16361,8 @@ dumpConstraint(Archive *fout, const ConstraintInfo
*coninfo)
}
else
{
- appendPQExpBuffer(q, "%s",
- coninfo->contype ==
'p' ? "PRIMARY KEY" : "UNIQUE");
+ appendPQExpBufferStr(q,
+
coninfo->contype == 'p' ? "PRIMARY KEY" : "UNIQUE");
if (indxinfo->indnullsnotdistinct)
appendPQExpBuffer(q, " NULLS NOT DISTINCT");
appendPQExpBuffer(q, " (");
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 9aadcaad71..c749349468 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3512,8 +3512,7 @@ printVerboseErrorMessages(CState *st, pg_time_usec_t
*now, bool is_retry)
resetPQExpBuffer(buf);
printfPQExpBuffer(buf, "client %d ", st->id);
- appendPQExpBuffer(buf, "%s",
- (is_retry ?
+ appendPQExpBufferStr(buf, (is_retry ?
"repeats the transaction after the
error" :
"ends the failed transaction"));
appendPQExpBuffer(buf, " (try %u", st->tries);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 61188d96f2..a141146e70 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4889,9 +4889,9 @@ pset_value_string(const char *param, printQueryOpt *popt)
else if (strcmp(param, "footer") == 0)
return pstrdup(pset_bool_string(popt->topt.default_footer));
else if (strcmp(param, "format") == 0)
- return psprintf("%s", _align2string(popt->topt.format));
+ return pstrdup(_align2string(popt->topt.format));
else if (strcmp(param, "linestyle") == 0)
- return psprintf("%s", get_line_style(&popt->topt)->name);
+ return pstrdup(get_line_style(&popt->topt)->name);
else if (strcmp(param, "null") == 0)
return pset_quoted_string(popt->nullPrint
?
popt->nullPrint
