From 372c3069b0f891eb60b63a4e407d32bb3f5a4df2 Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 3 Apr 2017 11:53:27 +0900 Subject: [PATCH] Fix how pg_dump emits partition attributes Currently, they will never be emitted inline, because a partition's attributes are *always* inherited (attislocal is never true in the case of partitions) and inherited attributes are not emitted. This means NOT NULL constraints and DEFAULT, if any, are needed to be emitted separately using ALTER TABLE ONLY ..., which is undesirable if the partition is itself partitioned (the ONLY part). Instead always emit them, so that NOT NULL and DEFAULT are included with CREATE TABLE. This patch includes one more fix: we should not emit WITH OPTIONS keyword phrase for partitions' columns. Also update some comments. --- src/bin/pg_dump/pg_dump.c | 49 +++++++++++++++++++++++++++++++++++++---------- src/bin/pg_dump/pg_dump.h | 1 + 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 65a2f2307a..512b45aa70 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -5457,6 +5457,7 @@ getTables(Archive *fout, int *numTables) int i_relpages; int i_is_identity_sequence; int i_changed_acl; + int i_relispartition; /* Make sure we are in proper schema */ selectSourceSchema(fout, "pg_catalog"); @@ -5533,7 +5534,22 @@ getTables(Archive *fout, int *numTables) "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text " "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, " "tc.reloptions AS toast_reloptions, " - "c.relkind = '%c' AND EXISTS (SELECT 1 FROM pg_depend WHERE classid = 'pg_class'::regclass AND objid = c.oid AND objsubid = 0 AND refclassid = 'pg_class'::regclass AND deptype = 'i') AS is_identity_sequence, " + "c.relkind = '%c' AND EXISTS (SELECT 1 FROM pg_depend WHERE classid = 'pg_class'::regclass AND objid = c.oid AND objsubid = 0 AND refclassid = 'pg_class'::regclass AND deptype = 'i') AS is_identity_sequence, ", + acl_subquery->data, + racl_subquery->data, + initacl_subquery->data, + initracl_subquery->data, + username_subquery, + RELKIND_SEQUENCE); + + /* + * Tables in >= PG 10 servers can be partitions, which need special + * handling when dumping attributes and attribute-level constraints. + */ + if (fout->remoteVersion >= 100000) + appendPQExpBuffer(query, "c.relispartition, "); + + appendPQExpBuffer(query, "EXISTS (SELECT 1 FROM pg_attribute at LEFT JOIN pg_init_privs pip ON " "(c.oid = pip.objoid " "AND pip.classoid = 'pg_class'::regclass " @@ -5558,12 +5574,6 @@ getTables(Archive *fout, int *numTables) "AND pip.objsubid = 0) " "WHERE c.relkind in ('%c', '%c', '%c', '%c', '%c', '%c', '%c') " "ORDER BY c.oid", - acl_subquery->data, - racl_subquery->data, - initacl_subquery->data, - initracl_subquery->data, - username_subquery, - RELKIND_SEQUENCE, attacl_subquery->data, attracl_subquery->data, attinitacl_subquery->data, @@ -5988,6 +5998,7 @@ 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_relispartition = PQfnumber(res, "relispartition"); if (dopt->lockWaitTimeout) { @@ -6057,6 +6068,10 @@ getTables(Archive *fout, int *numTables) else tblinfo[i].checkoption = pg_strdup(PQgetvalue(res, i, i_checkoption)); tblinfo[i].toast_reloptions = pg_strdup(PQgetvalue(res, i, i_toastreloptions)); + if (i_relispartition >= 0) + tblinfo[i].relispartition = (strcmp(PQgetvalue(res, i, i_relispartition), "t") == 0); + else + tblinfo[i].relispartition = false; /* other fields were zeroed above */ @@ -8207,7 +8222,8 @@ shouldPrintColumn(DumpOptions *dopt, TableInfo *tbinfo, int colno) { if (dopt->binary_upgrade) return true; - return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]); + return ((tbinfo->attislocal[colno] || tbinfo->relispartition) && + !tbinfo->attisdropped[colno]); } @@ -15141,6 +15157,11 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (tbinfo->reloftype && !dopt->binary_upgrade) appendPQExpBuffer(q, " OF %s", tbinfo->reloftype); + /* + * Create as a partition, if partitionOf is set; except in the case + * of a binary upgrade, we dump the table normally and attach it to + * the parent afterward. + */ if (tbinfo->partitionOf && !dopt->binary_upgrade) { TableInfo *parentRel = tbinfo->partitionOf; @@ -15212,11 +15233,17 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) continue; } - /* Attribute type */ + /* + * 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->partitionOf) && !dopt->binary_upgrade) { - appendPQExpBufferStr(q, " WITH OPTIONS"); + if (tbinfo->reloftype) + appendPQExpBufferStr(q, " WITH OPTIONS"); } else { @@ -15364,6 +15391,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 && diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 61097e6d99..e786499006 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 relispartition; /* is table a partition? */ /* * These fields are computed only if we decide the table is interesting -- 2.11.0