Hi Stephen,
Sorry about the delay.
On 2017/04/27 23:17, Stephen Frost wrote:
> Amit,
>
> * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
>>>> So to summarize what the patches do (some of these were posted earlier)
>>>>
>>>> 0002: pg_dump: Do not emit WITH OPTIONS keywords with partition's columns
>>>
>>> I'm trying to understand why this is also different. At least on an
>>> initial look, there seems to be a bunch more copy-and-paste from the
>>> existing TypedTable to Partition in gram.y, which seems to all boil down
>>> to removing the need for WITH OPTIONS to be specified. If WITH OPTIONS
>>> would be too much to have included for partitioning, and it isn't
>>> actually necessary, why not just make it optional, and use the same
>>> construct for both, removing all the duplicaiton and the need for
>>> pg_dump to output it?
>>
>> OK, I think optionally allowing WITH OPTIONS keywords would be nice.
>>
>> So in lieu of this patch, I propose a patch that modifies gram.y to allow
>> WITH OPTIONS to specified.
>
> The point I was trying to get at was that if you make WITH OPTIONS
> optional for the TypedTableElement case, you can remove all of the
> PartitionElement-related productions and then both the OF type_name and
> the PARTITION OF case will accept WITH OPTIONS as noise words and also
> work without WITH OPTIONS being specified.
>
> This also makes the code actually match the documentation since, at
> least in the CREATE FOREIGN TABLE documentation, we claim that WITH
> OPTIONS is required.
>
> Please see a proposed patch attached to accomplish this.
The committed patch looks good to me. Thanks.
Now that WITH OPTIONS is optional even for CREATE TABLE OF, perhaps it
needs to be mentioned in the release notes?
>>> Given that we're now setting numParents for partitions, I
>>> would think we'd just move the if (tbinfo->partitionOf) branches up
>>> under the if (numParents > 0) branches, which I think would also help us
>>> catch additional issues, like the fact that a binary-upgrade with
>>> partitions in a different namespace from the partitioned table would
>>> fail because the ATTACH PARTITION code doesn't account for it:
>>>
>>> appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
>>> fmtId(tbinfo->partitionOf->dobj.name));
>>> appendPQExpBuffer(q, "ATTACH PARTITION %s %s;\n",
>>> fmtId(tbinfo->dobj.name),
>>> tbinfo->partitiondef);
>>>
>>> unlike the existing inheritance code a few lines above, which does:
>>>
>>> appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
>>> fmtId(tbinfo->dobj.name));
>>> if (parentRel->dobj.namespace != tbinfo->dobj.namespace)
>>> appendPQExpBuffer(q, "%s.",
>>>
>>> fmtId(parentRel->dobj.namespace->dobj.name));
>>> appendPQExpBuffer(q, "%s;\n",
>>> fmtId(parentRel->dobj.name));
>>
>> That's a good point. I put both cases under the if (numParents > 0) block
>> and appropriate text is emitted depending on whether the table is a
>> partition or plain inheritance child.
>
> Right, ok.
>
>>>> 0004: Change the way pg_dump retrieves partitioning info (removes a lot
>>>> of unnecessary code and instead makes partitioning case use the old
>>>> inheritance code, etc.)
>>>
>>> This looks pretty good, at least on first blush, probably in part
>>> because it's mostly removing code. The CASE statement in the
>>> getTables() query isn't needed, once pg_get_partkeydef() has been
>>> changed to not throw an ERROR when passed a non-partitioned table OID.
>>
>> Oh yes, fixed.
>
> Good.
>
>> 0003: Change the way pg_dump retrieves partitioning info (removes a lot
>> of unnecessary code and instead makes partitioning case use the old
>> inheritance code, etc.)
>
> This has conflicts due to my proposed patch as my patch changes pg_dump
> to not output the now-noise-words WITH OPTIONS at all.
Rebased.
>> 0004: Fix a bug in pg_dump's --binary-upgrade code that caused ALTER TABLE
>> INHERIT to be emitted to attach a partition in addition to of ALTER
>> TABLE ATTACH PARTITION and that no schema-name was emitted where it
>> should have
>
> Given that it's touching the same places, this would really make sense
> to merge into 0003 now.
OK, done.
> I'm going to continue to mull over the attached for a bit and add some
> tests to it, but if it looks good then I'll push it this afternoon,
> after which you'll hopefully have time to rebase 0003 and merge in 0004
> to that, which I can review and probably push tomorrow.
Attached updated patches.
Thanks,
Amit
>From 41d8aa34c6a780fb3f46dc19f41b901e89be8726 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 18 Apr 2017 10:59:35 +0900
Subject: [PATCH 1/2] Improve test coverage of partition constraints
Better exercise the code manipulating partition constraints a bit
differently from the traditional inheritance children.
---
src/bin/pg_dump/t/002_pg_dump.pl | 10 +++++---
src/test/regress/expected/create_table.out | 41 ++++++++++++++++++++++--------
src/test/regress/sql/create_table.sql | 21 ++++++++++++---
3 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ccd0ed6a53..bebb2f4f5d 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4757,12 +4757,16 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
catch_all => 'CREATE ... commands',
create_order => 91,
create_sql => 'CREATE TABLE dump_test_second_schema.measurement_y2006m2
- PARTITION OF dump_test.measurement FOR VALUES
- FROM (\'2006-02-01\') TO (\'2006-03-01\');',
+ PARTITION OF dump_test.measurement (
+ unitsales DEFAULT 0 CHECK (unitsales >= 0)
+ )
+ FOR VALUES FROM (\'2006-02-01\') TO (\'2006-03-01\');',
regexp => qr/^
\Q-- Name: measurement_y2006m2;\E.*\n
\Q--\E\n\n
- \QCREATE TABLE measurement_y2006m2 PARTITION OF dump_test.measurement\E\n
+ \QCREATE TABLE measurement_y2006m2 PARTITION OF dump_test.measurement (\E\n
+ \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
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index dda0d7ee5d..79603166d1 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -618,16 +618,42 @@ CREATE TABLE part_b PARTITION OF parted (
) FOR VALUES IN ('b');
ERROR: column "b" specified more than once
CREATE TABLE part_b PARTITION OF parted (
- b NOT NULL DEFAULT 1 CHECK (b >= 0),
- CONSTRAINT check_a CHECK (length(a) > 0)
+ b NOT NULL DEFAULT 1,
+ CONSTRAINT check_a CHECK (length(a) > 0),
+ CONSTRAINT check_b CHECK (b >= 0)
) FOR VALUES IN ('b');
NOTICE: merging constraint "check_a" with inherited definition
-- conislocal should be false for any merged constraints
-SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass AND conname = 'check_a';
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
conislocal | coninhcount
------------+-------------
f | 1
-(1 row)
+ t | 0
+(2 rows)
+
+-- Once check_b is added to the parent, it should be made non-local for part_b
+ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0);
+NOTICE: merging constraint "check_b" with inherited definition
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
+ conislocal | coninhcount
+------------+-------------
+ f | 1
+ f | 1
+(2 rows)
+
+-- Neither check_a nor check_b are droppable from part_b
+ALTER TABLE part_b DROP CONSTRAINT check_a;
+ERROR: cannot drop inherited constraint "check_a" of relation "part_b"
+ALTER TABLE part_b DROP CONSTRAINT check_b;
+ERROR: cannot drop inherited constraint "check_b" of relation "part_b"
+-- And dropping it from parted should leave no trace of them on part_b, unlike
+-- traditional inheritance where they will be left behind, because they would
+-- be local constraints.
+ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b;
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
+ conislocal | coninhcount
+------------+-------------
+(0 rows)
-- specify PARTITION BY for a partition
CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c);
@@ -643,9 +669,6 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
a | text | | |
b | integer | | not null | 1
Partition of: parted FOR VALUES IN ('b')
-Check constraints:
- "check_a" CHECK (length(a) > 0)
- "part_b_b_check" CHECK (b >= 0)
-- Both partition bound and partition key in describe output
\d part_c
@@ -656,8 +679,6 @@ Check constraints:
b | integer | | not null | 0
Partition of: parted FOR VALUES IN ('c')
Partition key: RANGE (b)
-Check constraints:
- "check_a" CHECK (length(a) > 0)
Number of partitions: 1 (Use \d+ to list them.)
-- Show partition count in the parent's describe output
@@ -671,8 +692,6 @@ Number of partitions: 1 (Use \d+ to list them.)
a | text | | |
b | integer | | not null | 0
Partition key: LIST (a)
-Check constraints:
- "check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
-- cleanup
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index caf5ddb58b..14a8583c85 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -579,11 +579,26 @@ CREATE TABLE part_b PARTITION OF parted (
) FOR VALUES IN ('b');
CREATE TABLE part_b PARTITION OF parted (
- b NOT NULL DEFAULT 1 CHECK (b >= 0),
- CONSTRAINT check_a CHECK (length(a) > 0)
+ b NOT NULL DEFAULT 1,
+ CONSTRAINT check_a CHECK (length(a) > 0),
+ CONSTRAINT check_b CHECK (b >= 0)
) FOR VALUES IN ('b');
-- conislocal should be false for any merged constraints
-SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass AND conname = 'check_a';
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
+
+-- Once check_b is added to the parent, it should be made non-local for part_b
+ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0);
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
+
+-- Neither check_a nor check_b are droppable from part_b
+ALTER TABLE part_b DROP CONSTRAINT check_a;
+ALTER TABLE part_b DROP CONSTRAINT check_b;
+
+-- And dropping it from parted should leave no trace of them on part_b, unlike
+-- traditional inheritance where they will be left behind, because they would
+-- be local constraints.
+ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b;
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
-- specify PARTITION BY for a partition
CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c);
--
2.11.0
>From 4d823836b0e5855ec5e5e2d253400d4c43dd30bc Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 26 Apr 2017 14:18:21 +0900
Subject: [PATCH 2/2] Change the way pg_dump retrieves partitioning info
This gets rid of the code that issued separate queries to retrieve the
partitioning parent-child relationship, parent partition key, and child
partition bound information. With this patch, the information is
retrieved instead using the queries issued from getTables() and
getInherits(), which is both more efficient than the previous approach
and doesn't require any new code.
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.
Also, fix a bug in pg_dump's --binary-upgrade code, which caused pg_dump
to emit invalid command to attach a partition to its parent.
---
src/bin/pg_dump/common.c | 90 ----------------
src/bin/pg_dump/pg_dump.c | 214 ++++++++++++++-------------------------
src/bin/pg_dump/pg_dump.h | 15 +--
src/bin/pg_dump/t/002_pg_dump.pl | 2 -
4 files changed, 78 insertions(+), 243 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 2fda350faa..f8876856c2 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5507,6 +5507,25 @@ 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 = "pg_catalog.pg_get_partkeydef(c.oid)";
+ ispartition = "c.relispartition";
+ partbound = "pg_get_expr(c.relpartbound, c.oid)";
+ }
/* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog");
@@ -5594,7 +5613,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 +5640,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 +6063,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 +6168,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 +6298,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 +6325,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 +7693,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 +15116,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 +15164,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;
@@ -15276,7 +15201,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* partition ('PARTITION OF'), since the type comes from
* the parent/partitioned table.
*/
- if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->partitionOf))
+ if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->ispartition))
{
appendPQExpBuffer(q, " %s",
tbinfo->atttypnames[j]);
@@ -15330,7 +15255,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))
{
/*
@@ -15340,13 +15265,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++)
@@ -15497,13 +15425,33 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
TableInfo *parentRel = parents[k];
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
- fmtId(tbinfo->dobj.name));
+ /* In the partitioning case, we alter the parent */
+ if (tbinfo->ispartition)
+ appendPQExpBuffer(q, "ALTER TABLE ONLY %s ATTACH PARTITION ",
+ fmtId(parentRel->dobj.name));
+ else
+ appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
+ fmtId(tbinfo->dobj.name));
+
+ /* Add the appropriate schema-name if required */
if (parentRel->dobj.namespace != tbinfo->dobj.namespace)
- appendPQExpBuffer(q, "%s.",
+ {
+ if (tbinfo->ispartition)
+ appendPQExpBuffer(q, "%s.",
+ fmtId(tbinfo->dobj.namespace->dobj.name));
+ else
+ appendPQExpBuffer(q, "%s.",
fmtId(parentRel->dobj.namespace->dobj.name));
- appendPQExpBuffer(q, "%s;\n",
- fmtId(parentRel->dobj.name));
+ }
+
+ /* Partition needs specifying the bounds */
+ if (tbinfo->ispartition)
+ appendPQExpBuffer(q, "%s %s;\n",
+ fmtId(tbinfo->dobj.name),
+ tbinfo->partbound);
+ else
+ appendPQExpBuffer(q, "%s;\n",
+ fmtId(parentRel->dobj.name));
}
}
@@ -15515,16 +15463,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
tbinfo->reloftype);
}
- if (tbinfo->partitionOf)
- {
- appendPQExpBufferStr(q, "\n-- For binary upgrade, set up partitions this way.\n");
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
- fmtId(tbinfo->partitionOf->dobj.name));
- appendPQExpBuffer(q, "ATTACH PARTITION %s %s;\n",
- fmtId(tbinfo->dobj.name),
- tbinfo->partitiondef);
- }
-
appendPQExpBufferStr(q, "\n-- For binary upgrade, set heap's relfrozenxid and relminmxid\n");
appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
"SET relfrozenxid = '%u', relminmxid = '%u'\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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
