From 928377d4d2e37783b78e24771bc291f44d552d27 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 12 Apr 2017 14:53:09 +0900 Subject: [PATCH 2/3] Fix pg_dump to handle partition inheritance sanely With the current coding, partitions completely miss the flagInhAttr() treatment, which results in partition's attributes to be unnecessarily emitted. Update 002_pg_dump.pl to modify the expected output. --- src/bin/pg_dump/common.c | 19 ++++++++++++++----- src/bin/pg_dump/pg_dump.c | 36 ++++++++++++++++++++++++++---------- src/bin/pg_dump/pg_dump.h | 5 +++-- src/bin/pg_dump/t/002_pg_dump.pl | 2 -- 4 files changed, 43 insertions(+), 19 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index e2bc3576dc..8009b7c1e7 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -381,12 +381,15 @@ flagPartitions(TableInfo *tblinfo, int numTables, /* Find the parent TableInfo and save */ findPartitionParentByOid(&tblinfo[i], partinfo, numPartitions); - /* Mark the parent as interesting for getTableAttrs */ - if (tblinfo[i].partitionOf) + /* + * If a partition indeed, mark the only parent as interesting for + * getTableAttrs. + */ + if (tblinfo[i].partitiondef) { - tblinfo[i].partitionOf->interesting = true; + tblinfo[i].parents[0]->interesting = true; addObjectDependency(&tblinfo[i].dobj, - tblinfo[i].partitionOf->dobj.dumpId); + tblinfo[i].parents[0]->dobj.dumpId); } } } @@ -1008,6 +1011,12 @@ findPartitionParentByOid(TableInfo *self, PartInfo *partinfo, { TableInfo *parent; + /* Alright, self is a partition */ + self->ispartition = true; + + /* Partitions have exactly one parent. */ + self->numParents = 1; + self->parents = (TableInfo **) pg_malloc(sizeof(TableInfo *)); parent = findTableByOid(partinfo[i].partparent); if (parent == NULL) { @@ -1017,7 +1026,7 @@ findPartitionParentByOid(TableInfo *self, PartInfo *partinfo, oid); exit_nicely(1); } - self->partitionOf = parent; + self->parents[0] = parent; /* While we're at it, also save the partdef */ self->partitiondef = partinfo[i].partdef; diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5bd4af22c9..7560e528d2 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -15141,9 +15141,15 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (tbinfo->reloftype && !dopt->binary_upgrade) appendPQExpBuffer(q, " OF %s", tbinfo->reloftype); - if (tbinfo->partitionOf && !dopt->binary_upgrade) + /* + * 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->partitionOf; + /* Unlike the INHERITS case, only one parent here. */ + TableInfo *parentRel = tbinfo->parents[0]; appendPQExpBuffer(q, " PARTITION OF "); if (parentRel->dobj.namespace != tbinfo->dobj.namespace) @@ -15184,7 +15190,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) * Skip column if fully defined by reloftype or the * partition parent. */ - if ((tbinfo->reloftype || tbinfo->partitionOf) && + if ((tbinfo->reloftype || tbinfo->ispartition) && !has_default && !has_notnull && !dopt->binary_upgrade) continue; @@ -15212,8 +15218,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) continue; } - /* Attribute type */ - if ((tbinfo->reloftype || tbinfo->partitionOf) && + /* + * Attribute type. If this is not a binary upgrade dump, + * we need not dump the type for typed tables and + * partitions. Because in that case, we are merely + * dumping column's options, not defining a new column. + */ + if ((tbinfo->reloftype || tbinfo->ispartition) && !dopt->binary_upgrade) { appendPQExpBufferStr(q, " WITH OPTIONS"); @@ -15272,7 +15283,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (actual_atts) appendPQExpBufferStr(q, "\n)"); - else if (!((tbinfo->reloftype || tbinfo->partitionOf) && + else if (!((tbinfo->reloftype || tbinfo->ispartition) && !dopt->binary_upgrade)) { /* @@ -15282,13 +15293,15 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) appendPQExpBufferStr(q, " (\n)"); } - if (tbinfo->partitiondef && !dopt->binary_upgrade) + if (tbinfo->ispartition && !dopt->binary_upgrade) { appendPQExpBufferStr(q, "\n"); appendPQExpBufferStr(q, tbinfo->partitiondef); } - if (numParents > 0 && !dopt->binary_upgrade) + /* Emit the INHERITS clause unless this is a partition. */ + if (numParents > 0 && !tbinfo->ispartition && + !dopt->binary_upgrade) { appendPQExpBufferStr(q, "\nINHERITS ("); for (k = 0; k < numParents; k++) @@ -15364,6 +15377,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) * using an INHERITS clause --- the latter would possibly mess up the * column order. That also means we have to take care about setting * attislocal correctly, plus fix up any inherited CHECK constraints. + * The same applies to partitions, so we set up partitions using + * ALTER TABLE / ATTACH PARTITION instead of PARTITION OF. * Analogously, we set up typed tables using ALTER TABLE / OF here. */ if (dopt->binary_upgrade && @@ -15457,11 +15472,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) tbinfo->reloftype); } - if (tbinfo->partitionOf) + if (tbinfo->ispartition) { appendPQExpBufferStr(q, "\n-- For binary upgrade, set up partitions this way.\n"); + /* Note that there is only one parent in this case. */ appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - fmtId(tbinfo->partitionOf->dobj.name)); + fmtId(tbinfo->parents[0]->dobj.name)); appendPQExpBuffer(q, "ATTACH PARTITION %s %s;\n", fmtId(tbinfo->dobj.name), tbinfo->partitiondef); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 61097e6d99..10c3e109f5 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -295,6 +295,8 @@ typedef struct _tableInfo bool dummy_view; /* view's real definition must be postponed */ bool postponed_def; /* matview must be postponed into post-data */ + bool ispartition; /* is table a partition? */ + /* * These fields are computed only if we decide the table is interesting * (it's either a table to dump, or a direct parent of a dumpable table). @@ -329,8 +331,7 @@ typedef struct _tableInfo struct _tableDataInfo *dataObj; /* TableDataInfo, if dumping its data */ int numTriggers; /* number of triggers for table */ struct _triggerInfo *triggers; /* array of TriggerInfo structs */ - struct _tableInfo *partitionOf; /* TableInfo for the partition parent */ - char *partitiondef; /* partition key definition */ + char *partitiondef; /* partition bound definition */ } TableInfo; typedef struct _attrDefInfo diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index cccad04ab6..d01b1d25a2 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -4686,8 +4686,6 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog \Q--\E\n\n \QCREATE TABLE measurement_y2006m2 PARTITION OF dump_test.measurement\E\n \QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n - \QALTER TABLE ONLY measurement_y2006m2 ALTER COLUMN city_id SET NOT NULL;\E\n - \QALTER TABLE ONLY measurement_y2006m2 ALTER COLUMN logdate SET NOT NULL;\E\n /xm, like => { clean => 1, -- 2.11.0