From 8ac4fa1794eab6293a8a479d6005b131a0a4f418 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Thu, 7 Mar 2019 00:16:20 +1300 Subject: [PATCH v1] Make pg_dump emit ATTACH PARTITION instead of PARTITION OF ca41030 modified how the TABLESPACE option worked with partitioned tables so that it was possible to specify and store a TABLESPACE for a partitioned table. This setting served only to mark what the default tablespace should be for new partitions which were created with the CREATE TABLE PARTITION OF syntax. The problem with this was that pg_dump used the PARTITION OF syntax which meant that if a partition had been explicitly defined to have pg_default as the tablespace then since reltablespace is set to 0 in this case, pg_dump would emit a CREATE TABLE .. PARTITION OF statement and cause the partition to default to the partitioned table's tablespace rather than the one it was meant to be located in. We can work around this problem by simply performing the CREATE TABLE first then perform an ATTACH PARTITION later. This bypasses the check of the parent partitioned table's tablespace in DefineRelation() as the table is not a partition when it is defined. Doing this also means that when restoring partitions they now maintain their original column ordering rather than switch to their partitioned table's column ordering. This is perhaps minor, but it is noteworthy pg_dump in binary upgrade mode already worked this way, so it turns out this commit removes more code than it adds. --- src/bin/pg_dump/pg_dump.c | 114 +++++++++++++++++++--------------------------- 1 file changed, 47 insertions(+), 67 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5d83038348..08901372c9 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -8529,11 +8529,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) * Column number is zero-based. * * 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 + * 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.) - * 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. + * For partitions we must print the columns as we perform an ATTACH PARTITION + * after having performed CREATE TABLE. + * + * 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. * * This function exists because there are scattered nonobvious places that * must be kept in sync with this decision. @@ -8543,7 +8546,9 @@ shouldPrintColumn(DumpOptions *dopt, TableInfo *tbinfo, int colno) { if (dopt->binary_upgrade) return true; - return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]); + if (tbinfo->attisdropped[colno]) + return false; + return (tbinfo->attislocal[colno] || tbinfo->ispartition); } @@ -15503,27 +15508,6 @@ 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 */ @@ -15552,12 +15536,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) (!tbinfo->inhNotNull[j] || 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) + /* Skip column if fully defined by reloftype */ + if (tbinfo->reloftype && !has_default && !has_notnull && + !dopt->binary_upgrade) continue; /* Format properly if not first attr */ @@ -15580,7 +15561,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) * clean things up later. */ appendPQExpBufferStr(q, " INTEGER /* dummy */"); - /* Skip all the rest, too */ + /* and skip to the next column */ continue; } @@ -15589,11 +15570,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) * * 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. + * when creating a typed table ('OF type_name'). */ - if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->ispartition)) + if (dopt->binary_upgrade || !tbinfo->reloftype) { appendPQExpBuffer(q, " %s", tbinfo->atttypnames[j]); @@ -15643,23 +15622,19 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (actual_atts) appendPQExpBufferStr(q, "\n)"); - else if (!((tbinfo->reloftype || tbinfo->ispartition) && - !dopt->binary_upgrade)) + else if (!(tbinfo->reloftype && !dopt->binary_upgrade)) { /* - * We must have a parenthesized attribute list, even though - * empty, when not using the OF TYPE or PARTITION OF syntax. + * No attributes? we must have a parenthesized attribute list, + * even though empty, when not using the OF TYPE syntax. */ appendPQExpBufferStr(q, " (\n)"); } - if (tbinfo->ispartition && !dopt->binary_upgrade) - { - appendPQExpBufferChar(q, '\n'); - appendPQExpBufferStr(q, tbinfo->partbound); - } - - /* Emit the INHERITS clause, except if this is a partition. */ + /* + * Emit the INHERITS clause, except if this is a partition. When + * in binary upgrade mode we'll do this later in this function. + */ if (numParents > 0 && !tbinfo->ispartition && !dopt->binary_upgrade) @@ -15747,6 +15722,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) } } + /* Emit the ATTACH PARTITION clause */ + if (tbinfo->ispartition) + { + /* + * 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); + + /* Perform ALTER TABLE on the parent */ + appendPQExpBuffer(q, "ALTER TABLE ONLY %s ATTACH PARTITION ", + fmtQualifiedDumpable(parents[0])); + + /* specify the bounds */ + appendPQExpBuffer(q, "%s %s;\n", qualrelname, tbinfo->partbound); + } + /* * To create binary-compatible heap files, we have to ensure the same * physical column order, including dropped columns, as in the @@ -15834,30 +15828,16 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) appendPQExpBufferStr(q, "::pg_catalog.regclass;\n"); } - if (numParents > 0) + if (numParents > 0 && !tbinfo->ispartition) { - appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance and partitioning this way.\n"); + appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance this way.\n"); for (k = 0; k < numParents; k++) { TableInfo *parentRel = parents[k]; - /* 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)); + appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n", + qualrelname, + fmtQualifiedDumpable(parentRel)); } } -- 2.16.2.windows.1