From b4b94fdd94fd0814239c077fbbee6a7168f6ae7e Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 26 Apr 2017 14:18:21 +0900 Subject: [PATCH 4/5] Change the way pg_dump retrieves partitioning info Since partitioning parent-child relationship is now retrieved with the same old code that handles inheritance, partition attributes receive a proper flagInhAttrs() treatment (that it didn't receive before), which is needed so that the inherited NOT NULL constraints are not emitted if we already emitted it for the parent. --- src/bin/pg_dump/common.c | 90 ------------------ src/bin/pg_dump/pg_dump.c | 191 +++++++++++++-------------------------- src/bin/pg_dump/pg_dump.h | 15 +-- src/bin/pg_dump/t/002_pg_dump.pl | 2 - 4 files changed, 65 insertions(+), 233 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index e2bc3576dc..47191be86a 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -68,8 +68,6 @@ static int numextmembers; static void flagInhTables(TableInfo *tbinfo, int numTables, InhInfo *inhinfo, int numInherits); -static void flagPartitions(TableInfo *tblinfo, int numTables, - PartInfo *partinfo, int numPartitions); static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables); static DumpableObject **buildIndexArray(void *objArray, int numObjs, Size objSize); @@ -77,8 +75,6 @@ static int DOCatalogIdCompare(const void *p1, const void *p2); static int ExtensionMemberIdCompare(const void *p1, const void *p2); static void findParentsByOid(TableInfo *self, InhInfo *inhinfo, int numInherits); -static void findPartitionParentByOid(TableInfo *self, PartInfo *partinfo, - int numPartitions); static int strInArray(const char *pattern, char **arr, int arr_size); @@ -97,10 +93,8 @@ getSchemaData(Archive *fout, int *numTablesPtr) NamespaceInfo *nspinfo; ExtensionInfo *extinfo; InhInfo *inhinfo; - PartInfo *partinfo; int numAggregates; int numInherits; - int numPartitions; int numRules; int numProcLangs; int numCasts; @@ -238,10 +232,6 @@ getSchemaData(Archive *fout, int *numTablesPtr) inhinfo = getInherits(fout, &numInherits); if (g_verbose) - write_msg(NULL, "reading partition information\n"); - partinfo = getPartitions(fout, &numPartitions); - - if (g_verbose) write_msg(NULL, "reading event triggers\n"); getEventTriggers(fout, &numEventTriggers); @@ -255,11 +245,6 @@ getSchemaData(Archive *fout, int *numTablesPtr) write_msg(NULL, "finding inheritance relationships\n"); flagInhTables(tblinfo, numTables, inhinfo, numInherits); - /* Link tables to partition parents, mark parents as interesting */ - if (g_verbose) - write_msg(NULL, "finding partition relationships\n"); - flagPartitions(tblinfo, numTables, partinfo, numPartitions); - if (g_verbose) write_msg(NULL, "reading column info for interesting tables\n"); getTableAttrs(fout, tblinfo, numTables); @@ -293,10 +278,6 @@ getSchemaData(Archive *fout, int *numTablesPtr) getPolicies(fout, tblinfo, numTables); if (g_verbose) - write_msg(NULL, "reading partition key information for interesting tables\n"); - getTablePartitionKeyInfo(fout, tblinfo, numTables); - - if (g_verbose) write_msg(NULL, "reading publications\n"); getPublications(fout); @@ -354,43 +335,6 @@ flagInhTables(TableInfo *tblinfo, int numTables, } } -/* flagPartitions - - * Fill in parent link fields of every target table that is partition, - * and mark parents of partitions as interesting - * - * modifies tblinfo - */ -static void -flagPartitions(TableInfo *tblinfo, int numTables, - PartInfo *partinfo, int numPartitions) -{ - int i; - - for (i = 0; i < numTables; i++) - { - /* Some kinds are never partitions */ - if (tblinfo[i].relkind == RELKIND_SEQUENCE || - tblinfo[i].relkind == RELKIND_VIEW || - tblinfo[i].relkind == RELKIND_MATVIEW) - continue; - - /* Don't bother computing anything for non-target tables, either */ - if (!tblinfo[i].dobj.dump) - continue; - - /* Find the parent TableInfo and save */ - findPartitionParentByOid(&tblinfo[i], partinfo, numPartitions); - - /* Mark the parent as interesting for getTableAttrs */ - if (tblinfo[i].partitionOf) - { - tblinfo[i].partitionOf->interesting = true; - addObjectDependency(&tblinfo[i].dobj, - tblinfo[i].partitionOf->dobj.dumpId); - } - } -} - /* flagInhAttrs - * for each dumpable table in tblinfo, flag its inherited attributes * @@ -992,40 +936,6 @@ findParentsByOid(TableInfo *self, } /* - * findPartitionParentByOid - * find a partition's parent in tblinfo[] - */ -static void -findPartitionParentByOid(TableInfo *self, PartInfo *partinfo, - int numPartitions) -{ - Oid oid = self->dobj.catId.oid; - int i; - - for (i = 0; i < numPartitions; i++) - { - if (partinfo[i].partrelid == oid) - { - TableInfo *parent; - - parent = findTableByOid(partinfo[i].partparent); - if (parent == NULL) - { - write_msg(NULL, "failed sanity check, parent OID %u of table \"%s\" (OID %u) not found\n", - partinfo[i].partparent, - self->dobj.name, - oid); - exit_nicely(1); - } - self->partitionOf = parent; - - /* While we're at it, also save the partdef */ - self->partitiondef = partinfo[i].partdef; - } - } -} - -/* * parseOidArray * parse a string of numbers delimited by spaces into a character array * diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5f3ec51011..942c8f522e 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -5507,6 +5507,27 @@ getTables(Archive *fout, int *numTables) int i_relpages; int i_is_identity_sequence; int i_changed_acl; + int i_partkeydef; + int i_ispartition; + int i_partbound; + char *partkeydef; + char *ispartition; + char *partbound; + + if (fout->remoteVersion < 100000) + { + partkeydef = "NULL"; + ispartition = "NULL"; + partbound = "NULL"; + } + else + { + partkeydef = "CASE WHEN c.relkind = 'p' THEN" + " pg_catalog.pg_get_partkeydef(c.oid)" + " ELSE NULL END"; + ispartition = "c.relispartition"; + partbound = "pg_get_expr(c.relpartbound, c.oid)"; + } /* Make sure we are in proper schema */ selectSourceSchema(fout, "pg_catalog"); @@ -5594,7 +5615,10 @@ getTables(Archive *fout, int *numTables) "OR %s IS NOT NULL " "OR %s IS NOT NULL" "))" - "AS changed_acl " + "AS changed_acl, " + "%s AS partkeydef, " + "%s AS ispartition, " + "%s AS partbound " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -5618,6 +5642,9 @@ getTables(Archive *fout, int *numTables) attracl_subquery->data, attinitacl_subquery->data, attinitracl_subquery->data, + partkeydef, + ispartition, + partbound, RELKIND_SEQUENCE, RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW, RELKIND_COMPOSITE_TYPE, @@ -6038,6 +6065,9 @@ getTables(Archive *fout, int *numTables) i_reloftype = PQfnumber(res, "reloftype"); i_is_identity_sequence = PQfnumber(res, "is_identity_sequence"); i_changed_acl = PQfnumber(res, "changed_acl"); + i_partkeydef = PQfnumber(res, "partkeydef"); + i_ispartition = PQfnumber(res, "ispartition"); + i_partbound = PQfnumber(res, "partbound"); if (dopt->lockWaitTimeout) { @@ -6140,6 +6170,11 @@ getTables(Archive *fout, int *numTables) tblinfo[i].is_identity_sequence = (i_is_identity_sequence >= 0 && strcmp(PQgetvalue(res, i, i_is_identity_sequence), "t") == 0); + /* Partition key string or NULL*/ + tblinfo[i].partkeydef = pg_strdup(PQgetvalue(res, i, i_partkeydef)); + tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0); + tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound)); + /* * Read-lock target tables to make sure they aren't DROPPED or altered * in schema before we get around to dumping them. @@ -6265,11 +6300,7 @@ getInherits(Archive *fout, int *numInherits) * we want more information about partitions than just the parent-child * relationship. */ - appendPQExpBufferStr(query, - "SELECT inhrelid, inhparent " - "FROM pg_inherits " - "WHERE inhparent NOT IN (SELECT oid FROM pg_class " - "WHERE relkind = " CppAsString2(RELKIND_PARTITIONED_TABLE) ")"); + appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits"); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); @@ -6296,72 +6327,6 @@ getInherits(Archive *fout, int *numInherits) } /* - * getPartitions - * read all the partition inheritance and partition bound information - * from the system catalogs return them in the PartInfo* structure - * - * numPartitions is set to the number of pairs read in - */ -PartInfo * -getPartitions(Archive *fout, int *numPartitions) -{ - PGresult *res; - int ntups; - int i; - PQExpBuffer query; - PartInfo *partinfo; - - int i_partrelid; - int i_partparent; - int i_partbound; - - /* Before version 10, there are no partitions */ - if (fout->remoteVersion < 100000) - { - *numPartitions = 0; - return NULL; - } - - query = createPQExpBuffer(); - - /* Make sure we are in proper schema */ - selectSourceSchema(fout, "pg_catalog"); - - /* find the inheritance and boundary information about partitions */ - - appendPQExpBufferStr(query, - "SELECT inhrelid as partrelid, inhparent AS partparent," - " pg_get_expr(relpartbound, inhrelid) AS partbound" - " FROM pg_class c, pg_inherits" - " WHERE c.oid = inhrelid AND c.relispartition"); - - res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); - - ntups = PQntuples(res); - - *numPartitions = ntups; - - partinfo = (PartInfo *) pg_malloc(ntups * sizeof(PartInfo)); - - i_partrelid = PQfnumber(res, "partrelid"); - i_partparent = PQfnumber(res, "partparent"); - i_partbound = PQfnumber(res, "partbound"); - - for (i = 0; i < ntups; i++) - { - partinfo[i].partrelid = atooid(PQgetvalue(res, i, i_partrelid)); - partinfo[i].partparent = atooid(PQgetvalue(res, i, i_partparent)); - partinfo[i].partdef = pg_strdup(PQgetvalue(res, i, i_partbound)); - } - - PQclear(res); - - destroyPQExpBuffer(query); - - return partinfo; -} - -/* * getIndexes * get information about every index on a dumpable table * @@ -7730,49 +7695,6 @@ getTransforms(Archive *fout, int *numTransforms) } /* - * getTablePartitionKeyInfo - - * for each interesting partitioned table, read information about its - * partition key - * - * modifies tblinfo - */ -void -getTablePartitionKeyInfo(Archive *fout, TableInfo *tblinfo, int numTables) -{ - PQExpBuffer q; - int i; - PGresult *res; - - /* No partitioned tables before 10 */ - if (fout->remoteVersion < 100000) - return; - - q = createPQExpBuffer(); - - for (i = 0; i < numTables; i++) - { - TableInfo *tbinfo = &(tblinfo[i]); - - /* Only partitioned tables have partition key */ - if (tbinfo->relkind != RELKIND_PARTITIONED_TABLE) - continue; - - /* Don't bother computing anything for non-target tables, either */ - if (!tbinfo->dobj.dump) - continue; - - resetPQExpBuffer(q); - appendPQExpBuffer(q, "SELECT pg_catalog.pg_get_partkeydef('%u'::pg_catalog.oid)", - tbinfo->dobj.catId.oid); - res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); - Assert(PQntuples(res) == 1); - tbinfo->partkeydef = pg_strdup(PQgetvalue(res, 0, 0)); - } - - destroyPQExpBuffer(q); -} - -/* * getTableAttrs - * for each interesting table, read info about its attributes * (names, types, default values, CHECK constraints, etc) @@ -15196,9 +15118,14 @@ 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; + TableInfo *parentRel = tbinfo->parents[0]; appendPQExpBuffer(q, " PARTITION OF "); if (parentRel->dobj.namespace != tbinfo->dobj.namespace) @@ -15239,7 +15166,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; @@ -15267,8 +15194,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) { if (tbinfo->reloftype) @@ -15328,7 +15260,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)) { /* @@ -15338,13 +15270,16 @@ 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); + appendPQExpBufferStr(q, tbinfo->partbound); } - 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++) @@ -15489,7 +15424,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) } /* Note that partitions are handled differently (see below) */ - if (numParents > 0 && !tbinfo->partitionOf) + if (numParents > 0 && !tbinfo->ispartition) { appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance this way.\n"); for (k = 0; k < numParents; k++) @@ -15514,14 +15449,14 @@ 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"); 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); + tbinfo->partbound); } appendPQExpBufferStr(q, "\n-- For binary upgrade, set heap's relfrozenxid and relminmxid\n"); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 471cfce92a..4e6c83c943 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -294,6 +294,7 @@ typedef struct _tableInfo bool interesting; /* true if need to collect more data */ 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 @@ -319,6 +320,7 @@ typedef struct _tableInfo struct _attrDefInfo **attrdefs; /* DEFAULT expressions */ struct _constraintInfo *checkexprs; /* CHECK constraints */ char *partkeydef; /* partition key definition */ + char *partbound; /* partition bound definition */ bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */ /* @@ -329,8 +331,6 @@ 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 */ } TableInfo; typedef struct _attrDefInfo @@ -476,15 +476,6 @@ typedef struct _inhInfo Oid inhparent; /* OID of its parent */ } InhInfo; -/* PartInfo isn't a DumpableObject, just temporary state */ -typedef struct _partInfo -{ - Oid partrelid; /* OID of a partition */ - Oid partparent; /* OID of its parent */ - char *partdef; /* partition bound definition */ -} PartInfo; - - typedef struct _prsInfo { DumpableObject dobj; @@ -691,7 +682,6 @@ extern ConvInfo *getConversions(Archive *fout, int *numConversions); extern TableInfo *getTables(Archive *fout, int *numTables); extern void getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables); extern InhInfo *getInherits(Archive *fout, int *numInherits); -extern PartInfo *getPartitions(Archive *fout, int *numPartitions); extern void getIndexes(Archive *fout, TableInfo tblinfo[], int numTables); extern void getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables); extern void getConstraints(Archive *fout, TableInfo tblinfo[], int numTables); @@ -717,7 +707,6 @@ extern void processExtensionTables(Archive *fout, ExtensionInfo extinfo[], int numExtensions); extern EventTriggerInfo *getEventTriggers(Archive *fout, int *numEventTriggers); extern void getPolicies(Archive *fout, TableInfo tblinfo[], int numTables); -extern void getTablePartitionKeyInfo(Archive *fout, TableInfo *tblinfo, int numTables); extern void getPublications(Archive *fout); extern void getPublicationTables(Archive *fout, TableInfo tblinfo[], int numTables); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index bebb2f4f5d..7a003c7286 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -4768,8 +4768,6 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog \s+\QCONSTRAINT measurement_y2006m2_unitsales_check CHECK ((unitsales >= 0))\E\n \)\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