Hi Stephen,
On 2017/04/11 22:12, Stephen Frost wrote:
> Amit,
>
> * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
>> On 2017/04/11 0:26, Robert Haas wrote:
>>> Children can have constraints (including NOT NULL constraints) which
>>> parents lack, and can have a different column order, but must have
>>> exactly the same column names and types.
>>
>> Also, what is different in the partitioned parent case is that NOT NULL
>> constraints must be inherited. That is, one cannot add it only to the
>> parent.
>
> If I'm following, children can have additional constraints but any
> constraints on the parent must also exist on all the children. Is that
> correct?
Yes. As shown in the examples I posted, in traditional inheritance, only
CHECK constraints are treated that way, whereas NOT NULL constraints are
not. With partitioning, even the NOT NULL constraints are inherited. So
if a particular column is set NOT NULL in the parent, all partitions in
the tree must have that constraint and it cannot be dropped from a
partition without dropping it from the parent as well. Traditional
inheritance applies that rule only to attributes and CHECK constraints.
>> --
>> -- partitioning inheritance
>> --
>> create table parted_parent (a int) partition by list (a);
>> create table part partition of parted_parent for values in (1);
>>
>> -- this is same as traditional inheritance
>> alter table only parted_parent add constraint chka check (a > 0);
>> -- ERROR: constraint must be added to child tables too
>
> Ok, this makes sense, but surely that constraint does, in fact, exist on
> the child already or we wouldn't be trying to dump out this constraint
> that exists on the parent?
Yes, that's right.
Now that I have thought about this a bit more, I think I can articulate
the problem statement more clearly. The problem we have here is about
non-inherited constraints on partitions, specifically, the NOT NULL
constraints which need to be emitted per attribute. We have two options of
doing it: emit it inline with CREATE TABLE or separately with ALTER TABLE
ONLY.
I have been saying that it is preferable to use CREATE TABLE, because
ALTER TABLE ONLY will fail if the partition is itself partitioned. Why
does it cause an error? Because of the ONLY part. It is a user error to
specify ONLY when adding a constraint to a partitioned tables, but...
>>> In Amit's example from the original post, the child has an implicit
>>> NOT NULL constraint that does not exist in the parent. p1.b isn't
>>> declared NOT NULL, but the fact that it is range-partitioned on b
>>> requires it to be so, just as we would do if b were declared as the
>>> PRIMARY KEY. Somehow that's not playing nice with pg_dump, but I'm
>>> still fuzzy on the details.
>>
>> Actually, I would like to change the problem definition from "ALTER TABLE
>> ONLY partitioned_table should be avoided" to "Emitting partition's
>> attributes separately should be avoided".
>
> I don't follow why we think doing:
>
> CREATE TABLE t1 (c1 int);
> ALTER TABLE ONLY t1 SET c1 NOT NULL;
>
> is really different from:
>
> CREATE TABLE t1 (c1 int NOT NULL);
>
> or why we should teach pg_dump that it's "correct" to consider those two
> to be different. There are specific cases where they have to be done
> independently, but that's for views because we don't have a way to set a
> default on a view column during CREATE VIEW, or to deal with dropped
> columns or traditionally inheirited columns.
>
> What isn't clear to me is why the CREATE TABLE + ALTER TABLE isn't
> working, when apparently a CREATE TABLE with the NOT NULL included would
> work. The issue here seems like it's the order in which the operations
> are happening in, and not that CREATE TABLE + ALTER TABLE is somehow
> different than just the CREATE TABLE.
I think you are right. CREATE TABLE + ALTER TABLE ONLY should work just
fine in this particular case (i.e. the way pg_dump outputs it).
>> create table p (a int, b int) partition by list (a);
>> create table p1 partition of p for values in (1) partition by range (b);
>> create table p11 partition of p1 (
>> a not null default '1'
>> ) for values from (1) to (10);
>
> Using the above example, doing a pg_dump and then a restore (into a
> clean initdb'd cluster), I get the following:
>
> =# CREATE TABLE p (
> -# a integer,
> -# b integer
> -# )
> -# PARTITION BY LIST (a);
> CREATE TABLE
>
> =*# CREATE TABLE p1 PARTITION OF p
> -*# FOR VALUES IN (1)
> -*# PARTITION BY RANGE (b);
> CREATE TABLE
>
> =*# ALTER TABLE ONLY p1 ALTER COLUMN b SET NOT NULL;
> ERROR: constraint must be added to child tables too
>
> Now, perhaps I'm confused, but isn't p1 the child here? Which is
> supposed to be able to have constraints that the parent doesn't?
Actually, p1 is a partitioned table, so the error. And I realize that
that's a wrong behavior. Currently the check is performed using only the
relkind, which is bogus. Specifying ONLY should cause an error only when
the table has partitions.
> We haven't even gotten to the point where p1 is a parent yet because p11
> hasn't been created yet. Further, according to psql's \d, 'p1.b'
> already has a NOT NULL constraint on it, so the above really should just
> be a no-op.
Right. The already existing NOT NULL constraint results from p1.b being
the range partition key column.
> I get the feeling that we're looking in the wrong place for the issue
> here.
Yes, I think we should consider fixing the backend behavior here as Tom
also suspected. Throwing an error when no partitions yet exist might be a
bad idea after all.
Also, I mentioned above that the problem is only with non-inherited
constraints of partitions, but I've just discovered an issue with
inherited constraints as well. They need not be emitted for partitions
*again*, but they are.
So, here are the patches:
0001: Fix ALTER TABLE .. SET/DROP NOT NULL and DROP CONSTRAINT so that
they don't cause an error when ONLY is specified and no partitions exist.
0002: Fix pg_dump so that inherited constraints are not printed separately
for partitions. (pg_dump TAP test output is updated)
0003: Fix pg_dump to not emit WITH OPTIONS when printing constraints of
a partition's columns
It would be great if you could take a look.
Thanks,
Amit
>From 1bc9e9826b10bb162625a46c9cb15428637f8408 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 | 10 ------
src/backend/commands/tablecmds.c | 56 +++++++++++++++++++------------
src/test/regress/expected/alter_table.out | 30 +++++++++++------
src/test/regress/sql/alter_table.sql | 24 +++++++++----
4 files changed, 72 insertions(+), 48 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 340c961b3f..375debb25e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2950,16 +2950,6 @@ VALUES ('Albany', NULL, NULL, 'NY');
<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.
- </para>
- </listitem>
-
- <listitem>
- <para>
Partitions cannot have columns that are not present in the parent. It
is neither possible to specify columns when creating partitions with
<command>CREATE TABLE</> nor is it possible to add columns to
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a6a9f54b13..f9aba10bb6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5579,13 +5579,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 +5752,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
@@ -8558,16 +8569,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.
@@ -8577,6 +8578,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 2227f2d977..eff9615653 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -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/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8cd6786a90..23e6928b70 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);
--
2.11.0
>From 98305b43516b49c10f56e1d985d1502253237d32 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 65a2f2307a..b840a479e0 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 3782acebb0d3d8898d49734813d4474f72276bf9 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 b840a479e0..5ec8d9f3cf 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
