On 2017/04/13 6:22, Robert Haas wrote:
> On Wed, Apr 12, 2017 at 3:29 PM, Stephen Frost <sfr...@snowman.net> wrote:
>> I'm not following what you're getting at here.
>>
>> There's already a constraint on the table, and ALTER TABLE ONLY doesn't
>> say anything about what happens later on (certainly it doesn't make new
>> tables created with 'LIKE' have bits omitted, if that's what you were
>> thinking). Lastly, the error being thrown certainly seems to imply that
>> one needs to go fix all the child tables to have the constraint first
>> and then the constraint can be added to the parent (presumably using the
>> same ALTER TABLE ONLY command). If there aren't any child tables, then
>> it should work, if there *are* child tables and they've got the
>> necessary constraint, then this should be allowed, so that future child
>> tables create will have the constraint.
>
> So I think I was indeed confused before, and I think you're basically
> right here, but on one point I think you are not right -- ALTER TABLE
> ONLY .. CHECK () doesn't work on a table with inheritance children
> regardless of whether the children already have the matching
> constraint:
>
> rhaas=# create table foo (a int, b text);
> CREATE TABLE
> rhaas=# create table bar () inherits (foo);
> CREATE TABLE
> rhaas=# alter table only foo add check (a = 1);
> ERROR: constraint must be added to child tables too
> rhaas=# alter table only bar add check (a = 1);
> ALTER TABLE
> rhaas=# alter table only foo add check (a = 1);
> ERROR: constraint must be added to child tables too
>
> It looks like ALTER TABLE ONLY works find on a table with no children,
> but once it's got children it no longer works, period.
By the way, there is a workaround with traditional inheritance:
alter table only foo add constraint chka check (a > 0) no inherit;
ALTER TABLE
But we don't allow NO INHERIT constraints on partitioned tables, so we
will get an error with them anyway.
alter table only parted_parent add constraint chka check (a > 0) no inherit;
ERROR: cannot add NO INHERIT constraint to partitioned table "parted_parent"
> However,
> you're right that you can add the constraint to the as-yet-childless
> table and then future children will inherit the constraint properly.
> Continuing the previous example:
>
> rhaas=# drop table bar;
> DROP TABLE
> rhaas=# alter table only foo add check (a = 1);
> ALTER TABLE
> rhaas=# create table bar () inherits (foo);
> CREATE TABLE
>
> So, regarding Amit's 0001:
>
> - I think we should update the relevant hunk of the documentation
> rather than just removing it.
OK, I agree. I tweaked the existing bullet point about differences from
traditional inheritance when using ONLY with partitioned tables.
> - Should we similarly allow TRUNCATE ONLY foo and ALTER TABLE ONLY foo
> .. to work on a partitioned table without partitions, or is that just
> pointless tinkering? That seems to be the only case where, after this
> patch, an ONLY operation will fail on a childless partitioned table.
I fixed TRUNCATE ONLY to not complain when no partitions exist. Patch
already takes care of the ALTER TABLE ONLY cases.
Updated patches attached (0002 and 0003 unchanged).
Thanks,
Amit
>From 730a71a1120d37065e26338566fcbca65d1a03e8 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 12 Apr 2017 17:58:07 +0900
Subject: [PATCH 1/3] Fix ALTER TABLE ONLY to avoid unnecessarily failures
Currently, ALTER TABLE ONLY would fail in certain cases when applied
to partitioned tables, even if no partitions yet exist. Although,
it seems perfectly OK to allow the same, especially because pg_dump
sometimes outputs constraints separately using ALTER TABLE ONLY.
---
doc/src/sgml/ddl.sgml | 13 +++---
src/backend/commands/tablecmds.c | 69 +++++++++++++++++++------------
src/test/regress/expected/alter_table.out | 32 +++++++++-----
src/test/regress/expected/truncate.out | 5 +++
src/test/regress/sql/alter_table.sql | 24 ++++++++---
src/test/regress/sql/truncate.sql | 4 ++
6 files changed, 98 insertions(+), 49 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 340c961b3f..ae2072b1de 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2944,17 +2944,18 @@ VALUES ('Albany', NULL, NULL, 'NY');
Both <literal>CHECK</literal> and <literal>NOT NULL</literal>
constraints of a partitioned table are always inherited by all its
partitions. <literal>CHECK</literal> constraints that are marked
- <literal>NO INHERIT</literal> are not allowed.
+ <literal>NO INHERIT</literal> are not allowed to be created on
+ partitioned tables.
</para>
</listitem>
<listitem>
<para>
- The <literal>ONLY</literal> notation used to exclude child tables
- will cause an error for partitioned tables in the case of
- schema-modifying commands such as most <literal>ALTER TABLE</literal>
- commands. For example, dropping a column from only the parent does
- not make sense for partitioned tables.
+ Using <literal>ONLY</literal> to affect only the specified table
+ (and not its descendents) does not work with partitioned tables when
+ adding or dropping constraints (either <literal>CHECK</literal> or
+ <literal>NOT NULL</literal>) or with <command>TRUNCATE</command>, if
+ partitions exist.
</para>
</listitem>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a02904c85c..ae938c424f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1257,9 +1257,14 @@ ExecuteTruncate(TruncateStmt *stmt)
}
}
else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("must truncate child tables too")));
+ {
+ PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+
+ if (partdesc->nparts > 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("must truncate partitions too")));
+ }
}
/*
@@ -5579,13 +5584,19 @@ ATPrepDropNotNull(Relation rel, bool recurse, bool recursing)
{
/*
* If the parent is a partitioned table, like check constraints, NOT NULL
- * constraints must be dropped from child tables.
+ * constraints must be dropped from partitions. Complain if requested
+ * otherwise and partitions exist.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
- !recurse && !recursing)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("constraint must be dropped from child tables too")));
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+
+ Assert(partdesc != NULL);
+ if (partdesc->nparts > 0 && !recurse && !recursing)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("constraint must be dropped from partitions too")));
+ }
}
static ObjectAddress
ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
@@ -5746,13 +5757,18 @@ ATPrepSetNotNull(Relation rel, bool recurse, bool recursing)
{
/*
* If the parent is a partitioned table, like check constraints, NOT NULL
- * constraints must be added to the child tables.
+ * constraints must be added to the child tables. Complain if requested
+ * otherwise and partitions exist.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
- !recurse && !recursing)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("constraint must be added to child tables too")));
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+
+ if (partdesc && partdesc->nparts > 0 && !recurse && !recursing)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("constraint must be added to partitions too")));
+ }
}
static ObjectAddress
@@ -6547,7 +6563,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && !recurse)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("column must be dropped from child tables too")));
+ errmsg("column must be dropped from partitions too")));
attr_rel = heap_open(AttributeRelationId, RowExclusiveLock);
foreach(child, children)
@@ -8562,16 +8578,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
}
/*
- * In case of a partitioned table, the constraint must be dropped from the
- * partitions too. There is no such thing as NO INHERIT constraints in
- * case of partitioned tables.
- */
- if (!recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("constraint must be dropped from child tables too")));
-
- /*
* Propagate to children as appropriate. Unlike most other ALTER
* routines, we have to do this one level of recursion at a time; we can't
* use find_all_inheritors to do it in one pass.
@@ -8581,6 +8587,17 @@ ATExecDropConstraint(Relation rel, const char *constrName,
else
children = NIL;
+ /*
+ * For a partitioned table, if partitions exist and we are told not to
+ * recurse, it's a user error. It doesn't make sense to have a constraint
+ * be defined only on the parent, especially if it's a partitioned table.
+ */
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+ children != NIL && !recurse)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("constraint must be dropped from partitions too")));
+
foreach(child, children)
{
Oid childrelid = lfirst_oid(child);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 883a5c9864..23e7059cb6 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3295,7 +3295,7 @@ DROP TABLE part_3_4;
ALTER TABLE ONLY list_parted2 ADD COLUMN c int;
ERROR: column must be added to child tables too
ALTER TABLE ONLY list_parted2 DROP COLUMN b;
-ERROR: column must be dropped from child tables too
+ERROR: column must be dropped from partitions too
-- cannot add a column to partition or drop an inherited one
ALTER TABLE part_2 ADD COLUMN c text;
ERROR: cannot add column to a partition
@@ -3306,24 +3306,34 @@ ALTER TABLE part_2 RENAME COLUMN b to c;
ERROR: cannot rename inherited column "b"
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
ERROR: cannot alter inherited column "b"
--- cannot add NOT NULL or check constraints to *only* the parent (ie, non-inherited)
+-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- partitions exist
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
+ERROR: constraint must be added to partitions too
+ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
ERROR: constraint must be added to child tables too
-ALTER TABLE ONLY list_parted2 add constraint check_b check (b <> 'zz');
-ERROR: constraint must be added to child tables too
-ALTER TABLE list_parted2 add constraint check_b check (b <> 'zz') NO INHERIT;
-ERROR: cannot add NO INHERIT constraint to partitioned table "list_parted2"
+ALTER TABLE list_parted2 ALTER b SET NOT NULL;
+ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
+ERROR: constraint must be dropped from partitions too
+ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
+ERROR: constraint must be dropped from partitions too
+-- It's alright though, if no partitions are yet created
+CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
+ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
+ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
+ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
+ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
+DROP TABLE parted_no_parts;
-- cannot drop inherited NOT NULL or check constraints from partition
ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);
ALTER TABLE part_2 ALTER b DROP NOT NULL;
ERROR: column "b" is marked NOT NULL in parent table
ALTER TABLE part_2 DROP CONSTRAINT check_a2;
ERROR: cannot drop inherited constraint "check_a2" of relation "part_2"
--- cannot drop NOT NULL or check constraints from *only* the parent
-ALTER TABLE ONLY list_parted2 ALTER a DROP NOT NULL;
-ERROR: constraint must be dropped from child tables too
-ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_a2;
-ERROR: constraint must be dropped from child tables too
+-- Doesn't make sense to add NO INHERIT constraints on partitioned tables
+ALTER TABLE list_parted2 add constraint check_b2 check (b <> 'zz') NO INHERIT;
+ERROR: cannot add NO INHERIT constraint to partitioned table "list_parted2"
-- check that a partition cannot participate in regular inheritance
CREATE TABLE inh_test () INHERITS (part_2);
ERROR: cannot inherit from partition "part_2"
diff --git a/src/test/regress/expected/truncate.out b/src/test/regress/expected/truncate.out
index b652562f5b..8e1a08dd3a 100644
--- a/src/test/regress/expected/truncate.out
+++ b/src/test/regress/expected/truncate.out
@@ -452,7 +452,12 @@ LINE 1: SELECT nextval('truncate_a_id1');
^
-- partitioned table
CREATE TABLE truncparted (a int, b char) PARTITION BY LIST (a);
+-- no-op, because no partitions yet created
+TRUNCATE ONLY truncparted;
CREATE TABLE truncparted1 PARTITION OF truncparted FOR VALUES IN (1);
INSERT INTO truncparted VALUES (1, 'a');
+-- error, must truncate partitions
+TRUNCATE ONLY truncparted;
+ERROR: must truncate partitions too
TRUNCATE truncparted;
DROP TABLE truncparted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index eb1b4b536f..85c848f620 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2173,19 +2173,31 @@ ALTER TABLE part_2 DROP COLUMN b;
ALTER TABLE part_2 RENAME COLUMN b to c;
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
--- cannot add NOT NULL or check constraints to *only* the parent (ie, non-inherited)
+-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- partitions exist
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
-ALTER TABLE ONLY list_parted2 add constraint check_b check (b <> 'zz');
-ALTER TABLE list_parted2 add constraint check_b check (b <> 'zz') NO INHERIT;
+ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+
+ALTER TABLE list_parted2 ALTER b SET NOT NULL;
+ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
+ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
+
+-- It's alright though, if no partitions are yet created
+CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
+ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
+ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
+ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
+ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
+DROP TABLE parted_no_parts;
-- cannot drop inherited NOT NULL or check constraints from partition
ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);
ALTER TABLE part_2 ALTER b DROP NOT NULL;
ALTER TABLE part_2 DROP CONSTRAINT check_a2;
--- cannot drop NOT NULL or check constraints from *only* the parent
-ALTER TABLE ONLY list_parted2 ALTER a DROP NOT NULL;
-ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_a2;
+-- Doesn't make sense to add NO INHERIT constraints on partitioned tables
+ALTER TABLE list_parted2 add constraint check_b2 check (b <> 'zz') NO INHERIT;
-- check that a partition cannot participate in regular inheritance
CREATE TABLE inh_test () INHERITS (part_2);
diff --git a/src/test/regress/sql/truncate.sql b/src/test/regress/sql/truncate.sql
index 9d3d8de54a..c9f6e9fb18 100644
--- a/src/test/regress/sql/truncate.sql
+++ b/src/test/regress/sql/truncate.sql
@@ -236,7 +236,11 @@ SELECT nextval('truncate_a_id1'); -- fail, seq should have been dropped
-- partitioned table
CREATE TABLE truncparted (a int, b char) PARTITION BY LIST (a);
+-- no-op, because no partitions yet created
+TRUNCATE ONLY truncparted;
CREATE TABLE truncparted1 PARTITION OF truncparted FOR VALUES IN (1);
INSERT INTO truncparted VALUES (1, 'a');
+-- error, must truncate partitions
+TRUNCATE ONLY truncparted;
TRUNCATE truncparted;
DROP TABLE truncparted;
--
2.11.0
>From 928377d4d2e37783b78e24771bc291f44d552d27 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
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
>From c7ea303bf4453d1577699ca4349958be9c470436 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 12 Apr 2017 15:16:56 +0900
Subject: [PATCH 3/3] Do not emit WITH OPTIONS for partition's columns
CREATE TABLE OF requires it, but CREATE TABLE PARTITION OF doesn't.
---
src/bin/pg_dump/pg_dump.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7560e528d2..5e4af291f8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15227,7 +15227,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if ((tbinfo->reloftype || tbinfo->ispartition) &&
!dopt->binary_upgrade)
{
- appendPQExpBufferStr(q, " WITH OPTIONS");
+ if (tbinfo->reloftype)
+ appendPQExpBufferStr(q, " WITH OPTIONS");
}
else
{
--
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
