database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00
Code Activity

Revert "Make pg_dump emit ATTACH PARTITION instead of PARTITION OF"

Browse Source 
... and fallout (from branches 10, 11 and master).  The change was
ill-considered, and it broke a few normal use cases; since we don't have
time to fix it, we'll try again after this week's minor releases.

Reported-by: Rushabh Lathia
Discussion: https://postgr.es/m/CAGPqQf0iQV=PPOv2Btog9J9AwOQp6HmuVd6SbGTR_v3Zp2XT1w@mail.gmail.com
This commit is contained in:
Alvaro Herrera
2019-05-06 12:23:49 -04:00
parent f4540a9c9c
commit 40353bcc67
3 changed files with 78 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
doc/src/sgml/release-10.sgml
View File

@ -441,23 +441,6 @@
</para>
</listitem>
<listitem>
<para>
Make <application>pg_dump</application> recreate table partitions
using <command>ATTACH PARTITION</command> instead
of <command>CREATE TABLE ... PARTITION OF</command> (David Rowley)
</para>
<para>
This avoids various corner-case problems, notably that dump and
restore might unexpectedly alter a partition's column ordering.
It also means that a selective restore of the partition can succeed
even if its parent partitioned table isn't restored.
(The <command>ATTACH PARTITION</command> will fail of course, but
the partition table itself can be created and populated.)
</para>
</listitem>
<listitem>
<para>
Avoid crash in <filename>contrib/postgres_fdw</filename> when a

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

@ -8188,12 +8188,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
* Normally this is always true, but it's false for dropped columns, as well
* as those that were inherited without any local definition. (If we print
* such a column it will mistakenly get pg_attribute.attislocal set to true.)
* For partitions, it's always true, because we want the partitions to be
* created independently and ATTACH PARTITION used afterwards.
*
* In binary_upgrade mode, we must print all columns and fix the attislocal/
* attisdropped state later, so as to keep control of the physical column
* order.
* However, in binary_upgrade mode, we must print all such columns anyway and
* fix the attislocal/attisdropped state later, so as to keep control of the
* physical column order.
*
* This function exists because there are scattered nonobvious places that
* must be kept in sync with this decision.
@ -8203,9 +8200,7 @@ shouldPrintColumn(DumpOptions *dopt, TableInfo *tbinfo, int colno)
{
if (dopt->binary_upgrade)
return true;
if (tbinfo->attisdropped[colno])
return false;
return (tbinfo->attislocal[colno] || tbinfo->ispartition);
return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]);
}
@ -14968,6 +14963,27 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (tbinfo->reloftype && !dopt->binary_upgrade)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
/*
* If the table is a partition, dump it as such; except in the case of
* a binary upgrade, we dump the table normally and attach it to the
* parent afterward.
*/
if (tbinfo->ispartition && !dopt->binary_upgrade)
{
TableInfo *parentRel = tbinfo->parents[0];
/*
* With partitions, unlike inheritance, there can only be one
* parent.
*/
if (tbinfo->numParents != 1)
exit_horribly(NULL, "invalid number of parents %d for table \"%s\"\n",
tbinfo->numParents, tbinfo->dobj.name);
appendPQExpBuffer(q, " PARTITION OF %s",
fmtQualifiedDumpable(parentRel));
}
if (tbinfo->relkind != RELKIND_MATVIEW)
{
/* Dump the attributes */
@ -14996,9 +15012,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
(!tbinfo->inhNotNull[j] ||
dopt->binary_upgrade));
/* Skip column if fully defined by reloftype */
if (tbinfo->reloftype && !has_default && !has_notnull &&
!dopt->binary_upgrade)
/*
* Skip column if fully defined by reloftype or the
* partition parent.
*/
if ((tbinfo->reloftype || tbinfo->ispartition) &&
!has_default && !has_notnull && !dopt->binary_upgrade)
continue;
/* Format properly if not first attr */
@ -15021,16 +15040,20 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* clean things up later.
*/
appendPQExpBufferStr(q, " INTEGER /* dummy */");
/* and skip to the next column */
/* Skip all the rest, too */
continue;
}
/*
* Attribute type; print it except when creating a typed
* table ('OF type_name'), but in binary-upgrade mode,
* print it in that case too.
* Attribute type
*
* In binary-upgrade mode, we always include the type. If
* we aren't in binary-upgrade mode, then we skip the type
* when creating a typed table ('OF type_name') or a
* partition ('PARTITION OF'), since the type comes from
* the parent/partitioned table.
*/
if (dopt->binary_upgrade || !tbinfo->reloftype)
if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->ispartition))
{
appendPQExpBuffer(q, " %s",
tbinfo->atttypnames[j]);
@ -15080,20 +15103,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (actual_atts)
appendPQExpBufferStr(q, "\n)");
else if (!(tbinfo->reloftype && !dopt->binary_upgrade))
else if (!((tbinfo->reloftype || tbinfo->ispartition) &&
!dopt->binary_upgrade))
{
/*
* No attributes? we must have a parenthesized attribute list,
* even though empty, when not using the OF TYPE syntax.
* We must have a parenthesized attribute list, even though
* empty, when not using the OF TYPE or PARTITION OF syntax.
*/
appendPQExpBufferStr(q, " (\n)");
}
/*
* Emit the INHERITS clause (not for partitions), except in
* binary-upgrade mode.
*/
if (numParents > 0 && !tbinfo->ispartition &&
if (tbinfo->ispartition && !dopt->binary_upgrade)
{
appendPQExpBufferStr(q, "\n");
appendPQExpBufferStr(q, tbinfo->partbound);
}
/* Emit the INHERITS clause, except if this is a partition. */
if (numParents > 0 &&
!tbinfo->ispartition &&
!dopt->binary_upgrade)
{
appendPQExpBufferStr(q, "\nINHERITS (");
@ -15243,15 +15271,29 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
appendPQExpBufferStr(q, "::pg_catalog.regclass;\n");
}
if (numParents > 0 && !tbinfo->ispartition)
if (numParents > 0)
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance this way.\n");
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance and partitioning this way.\n");
for (k = 0; k < numParents; k++)
{
TableInfo *parentRel = parents[k];
appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n",
/* In the partitioning case, we alter the parent */
if (tbinfo->ispartition)
appendPQExpBuffer(q,
"ALTER TABLE ONLY %s ATTACH PARTITION ",
fmtQualifiedDumpable(parentRel));
else
appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
qualrelname);
/* Partition needs specifying the bounds */
if (tbinfo->ispartition)
appendPQExpBuffer(q, "%s %s;\n",
qualrelname,
tbinfo->partbound);
else
appendPQExpBuffer(q, "%s;\n",
fmtQualifiedDumpable(parentRel));
}
}
@ -15265,27 +15307,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
}
/*
* For partitioned tables, emit the ATTACH PARTITION clause. Note
* that we always want to create partitions this way instead of using
* CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
* layout discrepancy with the parent, but also to ensure it gets the
* correct tablespace setting if it differs from the parent's.
*/
if (tbinfo->ispartition)
{
/* With partitions there can only be one parent */
if (tbinfo->numParents != 1)
exit_horribly(NULL, "invalid number of parents %d for table \"%s\"\n",
tbinfo->numParents, tbinfo->dobj.name);
/* Perform ALTER TABLE on the parent */
appendPQExpBuffer(q,
"ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
fmtQualifiedDumpable(parents[0]),
qualrelname, tbinfo->partbound);
}
/*
* In binary_upgrade mode, arrange to restore the old relfrozenxid and
* relminmxid of all vacuumable relations. (While vacuum.c processes

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

@ -1047,8 +1047,8 @@ my %tests = (
\QALTER TABLE ONLY dump_test.measurement ATTACH PARTITION dump_test_second_schema.measurement_y2006m2 \E
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
/xm,
like => {
binary_upgrade => 1,
like => { binary_upgrade => 1, },
unlike => {
clean => 1,
clean_if_exists => 1,
createdb => 1,
@ -1057,14 +1057,13 @@ my %tests = (
exclude_test_table => 1,
exclude_test_table_data => 1,
no_blobs => 1,
no_owner => 1,
no_privs => 1,
no_owner => 1,
pg_dumpall_dbprivs => 1,
role => 1,
schema_only => 1,
section_pre_data => 1,
with_oids => 1,
}, unlike => {
only_dump_test_schema => 1,
only_dump_test_table => 1,
pg_dumpall_globals => 1,
@ -4834,8 +4833,7 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
\QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION OF dump_test.measurement\E\n
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
/xm,
like => {},
unlike => {
like => {
clean => 1,
clean_if_exists => 1,
createdb => 1,
@ -4850,7 +4848,8 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
role => 1,
schema_only => 1,
section_pre_data => 1,
with_oids => 1,
with_oids => 1, },
unlike => {
binary_upgrade => 1,
only_dump_test_schema => 1,
only_dump_test_table => 1,