sharpetronics/PostgreSQL
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-28 00:03:23 -04:00
Code Issues Projects Releases Wiki Activity

Reduce number of commands dumpTableSchema emits for binary upgrade.

Browse Source 
Avoid issuing a separate SQL UPDATE command for each column when
directly manipulating pg_attribute contents in binary upgrade mode.
With the separate updates, we triggered a relcache invalidation with
each update.  For a table with N columns, that causes O(N^2) relcache
bloat in txn_size mode because the table's newly-created relcache
entry can't be flushed till end of transaction.  Reducing the number
of commands should make it marginally faster as well as avoiding that
problem.

While at it, likewise avoid issuing a separate UPDATE on pg_constraint
for each inherited constraint.  This is less exciting, first because
inherited (non-partitioned) constraints are relatively rare, and
second because the backend has a good deal of trouble anyway with
restoring tables containing many such constraints, due to
MergeConstraintsIntoExisting being horribly inefficient.  But it seems
more consistent to do it this way here too, and it surely can't hurt.

In passing, fix one place in dumpTableSchema that failed to use ONLY
in ALTER TABLE.  That's not a live bug, but it's inconsistent.
Also avoid silently casting away const from string literals.

Per report from Justin Pryzby.  Back-patch to v17 where txn_size mode
was introduced.

Discussion: https://postgr.es/m/ZqEND4ZcTDBmcv31@pryzbyj2023
This commit is contained in:
Tom Lane 2024-07-29 11:53:49 -04:00
parent 0393f542d7
commit b3f0e0503f
2 changed files with 99 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
src/bin/pg_dump/pg_dump.c
View File

@ -15670,6 +15670,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
DumpOptions *dopt = fout->dopt;
PQExpBuffer q = createPQExpBuffer();
PQExpBuffer delq = createPQExpBuffer();
PQExpBuffer extra = createPQExpBuffer();
char *qrelname;
char *qualrelname;
int numParents;
@ -15736,7 +15737,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
char *partkeydef = NULL;
char *ftoptions = NULL;
char *srvname = NULL;
char *foreign = "";
const char *foreign = "";
/*
* Set reltypename, and collect any relkind-specific data that we
@ -16094,51 +16095,98 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
tbinfo->relkind == RELKIND_FOREIGN_TABLE ||
tbinfo->relkind == RELKIND_PARTITIONED_TABLE))
{
bool firstitem;
/*
* Drop any dropped columns. Merge the pg_attribute manipulations
* into a single SQL command, so that we don't cause repeated
* relcache flushes on the target table. Otherwise we risk O(N^2)
* relcache bloat while dropping N columns.
*/
resetPQExpBuffer(extra);
firstitem = true;
for (j = 0; j < tbinfo->numatts; j++)
{
if (tbinfo->attisdropped[j])
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, recreate dropped column.\n");
appendPQExpBuffer(q, "UPDATE pg_catalog.pg_attribute\n"
"SET attlen = %d, "
"attalign = '%c', attbyval = false\n"
"WHERE attname = ",
if (firstitem)
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, recreate dropped columns.\n"
"UPDATE pg_catalog.pg_attribute\n"
"SET attlen = v.dlen, "
"attalign = v.dalign, "
"attbyval = false\n"
"FROM (VALUES ");
firstitem = false;
}
else
appendPQExpBufferStr(q, ",\n ");
appendPQExpBufferChar(q, '(');
appendStringLiteralAH(q, tbinfo->attnames[j], fout);
appendPQExpBuffer(q, ", %d, '%c')",
tbinfo->attlen[j],
tbinfo->attalign[j]);
appendStringLiteralAH(q, tbinfo->attnames[j], fout);
appendPQExpBufferStr(q, "\n AND attrelid = ");
appendStringLiteralAH(q, qualrelname, fout);
appendPQExpBufferStr(q, "::pg_catalog.regclass;\n");
if (tbinfo->relkind == RELKIND_RELATION ||
tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
qualrelname);
else
appendPQExpBuffer(q, "ALTER FOREIGN TABLE ONLY %s ",
qualrelname);
appendPQExpBuffer(q, "DROP COLUMN %s;\n",
/* The ALTER ... DROP COLUMN commands must come after */
appendPQExpBuffer(extra, "ALTER %sTABLE ONLY %s ",
foreign, qualrelname);
appendPQExpBuffer(extra, "DROP COLUMN %s;\n",
fmtId(tbinfo->attnames[j]));
}
else if (!tbinfo->attislocal[j])
}
if (!firstitem)
{
appendPQExpBufferStr(q, ") v(dname, dlen, dalign)\n"
"WHERE attrelid = ");
appendStringLiteralAH(q, qualrelname, fout);
appendPQExpBufferStr(q, "::pg_catalog.regclass\n"
" AND attname = v.dname;\n");
/* Now we can issue the actual DROP COLUMN commands */
appendBinaryPQExpBuffer(q, extra->data, extra->len);
}
/*
* Fix up inherited columns. As above, do the pg_attribute
* manipulations in a single SQL command.
*/
firstitem = true;
for (j = 0; j < tbinfo->numatts; j++)
{
if (!tbinfo->attisdropped[j] &&
!tbinfo->attislocal[j])
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, recreate inherited column.\n");
appendPQExpBufferStr(q, "UPDATE pg_catalog.pg_attribute\n"
"SET attislocal = false\n"
"WHERE attname = ");
if (firstitem)
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, recreate inherited columns.\n");
appendPQExpBufferStr(q, "UPDATE pg_catalog.pg_attribute\n"
"SET attislocal = false\n"
"WHERE attrelid = ");
appendStringLiteralAH(q, qualrelname, fout);
appendPQExpBufferStr(q, "::pg_catalog.regclass\n"
" AND attname IN (");
firstitem = false;
}
else
appendPQExpBufferStr(q, ", ");
appendStringLiteralAH(q, tbinfo->attnames[j], fout);
appendPQExpBufferStr(q, "\n AND attrelid = ");
appendStringLiteralAH(q, qualrelname, fout);
appendPQExpBufferStr(q, "::pg_catalog.regclass;\n");
}
}
if (!firstitem)
appendPQExpBufferStr(q, ");\n");
/*
* Add inherited CHECK constraints, if any.
*
* For partitions, they were already dumped, and conislocal
* doesn't need fixing.
*
* As above, issue only one direct manipulation of pg_constraint.
* Although it is tempting to merge the ALTER ADD CONSTRAINT
* commands into one as well, refrain for now due to concern about
* possible backend memory bloat if there are many such
* constraints.
*/
resetPQExpBuffer(extra);
firstitem = true;
for (k = 0; k < tbinfo->ncheck; k++)
{
ConstraintInfo *constr = &(tbinfo->checkexprs[k]);
@ -16146,18 +16194,31 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
if (constr->separate || constr->conislocal || tbinfo->ispartition)
continue;
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraint.\n");
if (firstitem)
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraints.\n");
appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ADD CONSTRAINT %s %s;\n",
foreign, qualrelname,
fmtId(constr->dobj.name),
constr->condef);
appendPQExpBufferStr(q, "UPDATE pg_catalog.pg_constraint\n"
"SET conislocal = false\n"
"WHERE contype = 'c' AND conname = ");
appendStringLiteralAH(q, constr->dobj.name, fout);
appendPQExpBufferStr(q, "\n AND conrelid = ");
appendStringLiteralAH(q, qualrelname, fout);
appendPQExpBufferStr(q, "::pg_catalog.regclass;\n");
/* Update pg_constraint after all the ALTER TABLEs */
if (firstitem)
{
appendPQExpBufferStr(extra, "UPDATE pg_catalog.pg_constraint\n"
"SET conislocal = false\n"
"WHERE contype = 'c' AND conrelid = ");
appendStringLiteralAH(extra, qualrelname, fout);
appendPQExpBufferStr(extra, "::pg_catalog.regclass\n");
appendPQExpBufferStr(extra, " AND conname IN (");
firstitem = false;
}
else
appendPQExpBufferStr(extra, ", ");
appendStringLiteralAH(extra, constr->dobj.name, fout);
}
if (!firstitem)
{
appendPQExpBufferStr(extra, ");\n");
appendBinaryPQExpBuffer(q, extra->data, extra->len);
}
if (numParents > 0 && !tbinfo->ispartition)
@ -16344,7 +16405,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
if (tbinfo->relkind == RELKIND_FOREIGN_TABLE &&
tbinfo->attfdwoptions[j][0] != '\0')
appendPQExpBuffer(q,
"ALTER FOREIGN TABLE %s ALTER COLUMN %s OPTIONS (\n"
"ALTER FOREIGN TABLE ONLY %s ALTER COLUMN %s OPTIONS (\n"
" %s\n"
");\n",
qualrelname,
@ -16445,6 +16506,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
destroyPQExpBuffer(q);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(extra);
free(qrelname);
free(qualrelname);
}

2
src/bin/pg_dump/t/002_pg_dump.pl
View File

@ -1154,7 +1154,7 @@ my %tests = (
'ALTER FOREIGN TABLE foreign_table ALTER COLUMN c1 OPTIONS' => {
regexp => qr/^
\QALTER FOREIGN TABLE dump_test.foreign_table ALTER COLUMN c1 OPTIONS (\E\n
\QALTER FOREIGN TABLE ONLY dump_test.foreign_table ALTER COLUMN c1 OPTIONS (\E\n
\s+\Qcolumn_name 'col1'\E\n
\Q);\E\n
/xm,