On Thu, Dec 21, 2023 at 4:32 PM Peter Eisentraut <pe...@eisentraut.org> wrote: > > On 19.12.23 11:47, Ashutosh Bapat wrote: > > At this point I am looking for opinions on the above rules and whether > > the implementation is on the right track. > > This looks on the right track to me.
Thanks. > > > 0001 - change to get_partition_ancestors() prologue. Can be reviewed > > and committed independent of other patches. > > I committed that. Thanks. > > > 0004 - An attached partition inherits identity property and uses the > > underlying sequence for direct INSERTs. When inheriting the identity > > property it should also inherit the NOT NULL constraint, but that's a > > TODO in this patch. We expect matching NOT NULL constraints to be > > present in the partition being attached. I am not sure whether we want > > to add NOT NULL constraints automatically for an identity column. We > > require a NOT NULL constraint to be present when adding identity > > property to a column. The behavior in the patch seems to be consistent > > with this. > > I think it makes sense that the NOT NULL constraint must be added > manually before attaching is allowed. > Ok. I have modified the test case to add NOT NULL constraint. Here's complete patch-set. 0001 - fixes unrelated documentation style - can be committed independently OR ignored 0002 - adds an Assert in related code - can be independently committed On Mon, Nov 13, 2023 at 3:51 PM Peter Eisentraut <pe...@eisentraut.org> wrote: > Note, here is a writeup about the behavior of generated columns with > partitioning: > https://www.postgresql.org/docs/devel/ddl-generated-columns.html. It > would be useful if we documented the behavior of identity columns > similarly. (I'm not saying the behavior has to match.) 0003 - addresses this request 0004 - 0011 - each patch contains code changes and SQL testing those changes for ease of review. Each patch has commit message that describes the changes and rationale, if any, behind those changes. 0012 - test changes 0013 - expected output change because of code changes All these patches should be committed as a single commit finally. Please let me know when I can squash those all together. We may commit 0003 separately or along with 0004-0013. 0014 and 0015 - pg_dump/restore and pg_upgrade tests. But these patches are not expected to be committed for the reasons explained in the commit message. Since identity columns of a partitioned table are not marked as such in partitions in the older version, I tested their upgrade from PG 14 through the changes in 0015. pg_dumpall_14.out contains the dump file from PG 14 I used for this testing. -- Best Wishes, Ashutosh Bapat
From fee416ae2141019b7370a2ae31fc3d927d639977 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Tue, 9 Jan 2024 15:42:03 +0530 Subject: [PATCH 03/27] Add Identity Column section under Data Definition chapter This seems to be missing since identity column support was added. Add the same. Ashutosh Bapat --- doc/src/sgml/ddl.sgml | 56 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index bb6ce9291e..b33c35e141 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -404,6 +404,62 @@ CREATE TABLE people ( </para> </sect1> + <sect1 id="ddl-identity-columns"> + <title>Identity Columns</title> + + <indexterm zone="ddl-identity-columns"> + <primary>identity column</primary> + </indexterm> + + <para> + An identity column is a special column that is generated automatically. It + can be used to generate key values. In <productname>PostgreSQL</productname> + each identity column has an associated sequence from which it derives its + values. Such a column is implicitly NOT NULL. An identity column, however, + does not guarantee uniqueness. Uniqueness must be enforced using a + <literal>PRIMARY KEY</literal> or <literal>UNIQUE</literal> constraint or a + <literal>UNIQUE</literal> index. + </para> + + <para> + To create an identity column, use the <literal>GENERATED ... + AS IDENTITY</literal> clause in <command>CREATE TABLE</command>, for example: +<programlisting> +CREATE TABLE people ( + id bigint <emphasis>GENERATED ALWAYS AS IDENTITY</emphasis>, + ..., +); +</programlisting> + See <xref linkend="sql-createtable"/> for more details. The data type of an + identity column must be one of the data types supported by sequences. See + <xref linkend="sql-createsequence"/>. The properties of associated sequence + may be specified when creating an identity column (See <xref + linkend="sql-createtable"/>) or changed afterwards (See <xref + linkend="sql-altertable"/>). + </para> + + <para> + In <command>INSERT</command> or <command>UPDATE</command> commands, the + keyword <literal>DEFAULT</literal> may be used, if necessary, to specify + system generated value. + </para> + + <para> + Identity columns and their properties in a child table are independent of + those in its parent tables. A child table does not inherit identity columns + or their properties automatically from the parent. During insert or update, a + column is treated as an identity column if that column is an identity column + in the table named in the query and corresponding identity properties are + applied. + </para> + + <para> + Partitions inherit indentity columns from the partitioned table. They cannot + have their own identity columns. The properties of a given identity column + are consistent across all the partitions in the partition hierarchy. + </para> + </sect1> + <sect1 id="ddl-constraints"> <title>Constraints</title> -- 2.25.1
From 428d997b9f0172d1cdea02a8946843dcf474c759 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 7 Dec 2023 11:38:07 +0530
Subject: [PATCH 02/27] Assert that partition inherits from only one parent in
MergeAttributes()
A partition inherits only from one partitioned table and thus inherits
column definition only once. Assert the same in MergeAttributes() and
simplify a condition accordingly.
Similar definition exists about line 3068 in the same function.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6b0a20010e..18046a0d69 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2712,6 +2712,12 @@ MergeAttributes(List *columns, const List *supers, char relpersistence,
int32 deftypmod;
Oid defCollId;
+ /*
+ * Partitions have only one parent and have no column
+ * definitions of their own, so conflict should never occur.
+ */
+ Assert(!is_partition);
+
/*
* Yes, try to merge the two column definitions.
*/
@@ -2783,12 +2789,9 @@ MergeAttributes(List *columns, const List *supers, char relpersistence,
/*
* In regular inheritance, columns in the parent's primary key
- * get an extra not-null constraint. Partitioning doesn't
- * need this, because the PK itself is going to be cloned to
- * the partition.
+ * get an extra not-null constraint.
*/
- if (!is_partition &&
- bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber,
+ if (bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber,
pkattrs))
{
CookedConstraint *nn;
--
2.25.1
From 4d2dbe95f3c5267d7ddf7144329c5a98d95842f9 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 6 Dec 2023 16:08:59 +0530
Subject: [PATCH 05/27] Attached partition inherits identity column
A table being attached as a partition inherits identity property from
the partitioned table. This should be fine since we expect that the
partition table's column has the same type as the partitioned table's
corresponding column. If the table being attached is a partitioned
table, the indentity properties are propagated down its partition
hierarchy.
An Identity column in the partitioned table is also marked as NOT NULL.
The corresponding column in the partition needs to be marked as NOT NULL
for the attach to succeed.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 25 ++++++++++++++++++-------
src/test/regress/expected/identity.out | 23 ++++++++++++++++++-----
src/test/regress/sql/identity.sql | 9 +++++++++
3 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 06f6301a57..5719df2b76 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -354,7 +354,8 @@ static List *MergeAttributes(List *columns, const List *supers, char relpersiste
bool is_partition, List **supconstr,
List **supnotnulls);
static List *MergeCheckConstraint(List *constraints, const char *name, Node *expr);
-static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
+static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel,
+ bool ispartition);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
static void StoreCatalogInheritance(Oid relationId, List *supers,
bool child_is_partition);
@@ -618,7 +619,8 @@ static PartitionSpec *transformPartitionSpec(Relation rel, PartitionSpec *partsp
static void ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNumber *partattrs,
List **partexprs, Oid *partopclass, Oid *partcollation,
PartitionStrategy strategy);
-static void CreateInheritance(Relation child_rel, Relation parent_rel);
+static void CreateInheritance(Relation child_rel, Relation parent_rel,
+ bool ispartition);
static void RemoveInheritance(Relation child_rel, Relation parent_rel,
bool expect_detached);
static void ATInheritAdjustNotNulls(Relation parent_rel, Relation child_rel,
@@ -15647,7 +15649,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
errdetail("ROW triggers with transition tables are not supported in inheritance hierarchies.")));
/* OK to create inheritance */
- CreateInheritance(child_rel, parent_rel);
+ CreateInheritance(child_rel, parent_rel, false);
/*
* If parent_rel has a primary key, then child_rel has not-null
@@ -15673,7 +15675,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
* Common to ATExecAddInherit() and ATExecAttachPartition().
*/
static void
-CreateInheritance(Relation child_rel, Relation parent_rel)
+CreateInheritance(Relation child_rel, Relation parent_rel, bool ispartition)
{
Relation catalogRelation;
SysScanDesc scan;
@@ -15718,7 +15720,7 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
systable_endscan(scan);
/* Match up the columns and bump attinhcount as needed */
- MergeAttributesIntoExisting(child_rel, parent_rel);
+ MergeAttributesIntoExisting(child_rel, parent_rel, ispartition);
/* Match up the constraints and bump coninhcount as needed */
MergeConstraintsIntoExisting(child_rel, parent_rel);
@@ -15796,7 +15798,8 @@ constraints_equivalent(HeapTuple a, HeapTuple b, TupleDesc tupleDesc)
* the child must be as well. Defaults are not compared, however.
*/
static void
-MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
+MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel,
+ bool ispartition)
{
Relation attrrel;
TupleDesc parent_desc;
@@ -15865,6 +15868,14 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("column \"%s\" in child table must not be a generated column", parent_attname)));
+ /*
+ * Regular inheritance children are independent enough not to
+ * inherit identity columns. But partitions are integral part of a
+ * partitioned table and inherit identity column.
+ */
+ if (ispartition)
+ child_att->attidentity = parent_att->attidentity;
+
/*
* OK, bump the child column's inheritance count. (If we fail
* later on, this change will just roll back.)
@@ -18696,7 +18707,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
cmd->bound, pstate);
/* OK to create inheritance. Rest of the checks performed there */
- CreateInheritance(attachrel, rel);
+ CreateInheritance(attachrel, rel, true);
/* Update the pg_class entry. */
StorePartitionBound(attachrel, rel, cmd->bound);
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 20803a1d24..222b3684da 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -542,15 +542,28 @@ DROP TYPE itest_type CASCADE;
-- table partitions
-- partitions inherit identity column and share sequence
CREATE TABLE pitest1 (f1 date NOT NULL, f2 text, f3 bigint generated always as identity) PARTITION BY RANGE (f1);
+-- new partition
CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
+-- attached partition
+CREATE TABLE pitest1_p2 (f1 date NOT NULL, f2 text, f3 bigint);
+INSERT INTO pitest1_p2 VALUES ('2016-08-2', 'before attaching', 100);
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01'); -- requires NOT NULL constraint
+ERROR: column "f3" in child table must be marked NOT NULL
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET NOT NULL;
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
+INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest1_p2');
+INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
- tableoid | f1 | f2 | f3
-------------+------------+-----------------+----
- pitest1_p1 | 07-02-2016 | from pitest1 | 1
- pitest1_p1 | 07-03-2016 | from pitest1_p1 | 2
-(2 rows)
+ tableoid | f1 | f2 | f3
+------------+------------+------------------+-----
+ pitest1_p1 | 07-02-2016 | from pitest1 | 1
+ pitest1_p1 | 07-03-2016 | from pitest1_p1 | 2
+ pitest1_p2 | 08-02-2016 | before attaching | 100
+ pitest1_p2 | 08-03-2016 | from pitest1_p2 | 3
+ pitest1_p2 | 08-04-2016 | from pitest1 | 4
+(5 rows)
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 3660fa6a28..be29cbf119 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -335,9 +335,18 @@ DROP TYPE itest_type CASCADE;
-- partitions inherit identity column and share sequence
CREATE TABLE pitest1 (f1 date NOT NULL, f2 text, f3 bigint generated always as identity) PARTITION BY RANGE (f1);
+-- new partition
CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
+-- attached partition
+CREATE TABLE pitest1_p2 (f1 date NOT NULL, f2 text, f3 bigint);
+INSERT INTO pitest1_p2 VALUES ('2016-08-2', 'before attaching', 100);
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01'); -- requires NOT NULL constraint
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET NOT NULL;
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
+INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest1_p2');
+INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
-- partition with identity column of its own is not allowed
--
2.25.1
From 18ac85e9467038ec4cce03162f2d91e2efaa6bc4 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Tue, 5 Dec 2023 17:08:11 +0530
Subject: [PATCH 04/27] A newly created partition inherits indentity property
The partitions of a partitioned table are integral part of the
partitioned table. A partition inherits identity columns from the
partitioned table. An indentity column of a partition shares the
identity space with the corresponding column of the partitioned table.
In other words, the same identity column across all partitions of a
partitioned table share the same identity space. This is effected by
sharing the same underlying sequence.
When INSERTing directly into a partition, sequence associated with the
topmost partitioned table is used to calculate the value of the
corresponding identity column.
In regular inheritance, Identity columns and their properties in a child
table are independent of those in its parent tables. A child table does
not inherit identity columns or their properties automatically from the
parent.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 9 +++++++++
src/backend/rewrite/rewriteHandler.c | 19 ++++++++++++++++++-
src/test/regress/expected/identity.out | 15 ++++++++++++++-
src/test/regress/sql/identity.sql | 10 +++++++++-
4 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 18046a0d69..06f6301a57 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2855,6 +2855,15 @@ MergeAttributes(List *columns, const List *supers, char relpersistence,
def->is_not_null = true;
def->storage = attribute->attstorage;
def->generated = attribute->attgenerated;
+
+ /*
+ * Regular inheritance children are independent enough not to
+ * inherit identity columns. But partitions are integral part
+ * of a partitioned table and inherit identity column.
+ */
+ if (is_partition)
+ def->identity = attribute->attidentity;
+
if (CompressionMethodIsValid(attribute->attcompression))
def->compression =
pstrdup(GetCompressionMethodName(attribute->attcompression));
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 41a362310a..d5f1993665 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -25,6 +25,7 @@
#include "access/table.h"
#include "catalog/dependency.h"
#include "catalog/pg_type.h"
+#include "catalog/partition.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "foreign/fdwapi.h"
@@ -1234,8 +1235,24 @@ build_column_default(Relation rel, int attrno)
if (att_tup->attidentity)
{
NextValueExpr *nve = makeNode(NextValueExpr);
+ Oid reloid;
- nve->seqid = getIdentitySequence(RelationGetRelid(rel), attrno, false);
+ /*
+ * The identity sequence is associated with the topmost partitioned
+ * table.
+ */
+ if (rel->rd_rel->relispartition)
+ {
+ List *ancestors =
+ get_partition_ancestors(RelationGetRelid(rel));
+
+ reloid = llast_oid(ancestors);
+ list_free(ancestors);
+ }
+ else
+ reloid = RelationGetRelid(rel);
+
+ nve->seqid = getIdentitySequence(reloid, attrno, false);
nve->typeId = att_tup->atttypid;
return (Node *) nve;
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 7c6e87e8a5..20803a1d24 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -539,7 +539,20 @@ CREATE TYPE itest_type AS (f1 integer, f2 text, f3 bigint);
CREATE TABLE itest12 OF itest_type (f1 WITH OPTIONS GENERATED ALWAYS AS IDENTITY); -- error
ERROR: identity columns are not supported on typed tables
DROP TYPE itest_type CASCADE;
--- table partitions (currently not supported)
+-- table partitions
+-- partitions inherit identity column and share sequence
+CREATE TABLE pitest1 (f1 date NOT NULL, f2 text, f3 bigint generated always as identity) PARTITION BY RANGE (f1);
+CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
+INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+----
+ pitest1_p1 | 07-02-2016 | from pitest1 | 1
+ pitest1_p1 | 07-03-2016 | from pitest1_p1 | 2
+(2 rows)
+
+-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 9b8db2e4a3..3660fa6a28 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -331,8 +331,16 @@ CREATE TABLE itest12 OF itest_type (f1 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
DROP TYPE itest_type CASCADE;
--- table partitions (currently not supported)
+-- table partitions
+-- partitions inherit identity column and share sequence
+CREATE TABLE pitest1 (f1 date NOT NULL, f2 text, f3 bigint generated always as identity) PARTITION BY RANGE (f1);
+CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
+INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
+
+-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
--
2.25.1
From 2fa0310839eae93a1bbf8c9ab3a4cea3dd9d635b Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Tue, 9 Jan 2024 15:40:27 +0530
Subject: [PATCH 01/27] Decorate PostgreSQL with productname tag
... in the section about Generated Columns.
Ashutosh Bapat
---
doc/src/sgml/ddl.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 22d04006ad..bb6ce9291e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -249,7 +249,7 @@ CREATE TABLE products (
storage and is computed when it is read. Thus, a virtual generated column
is similar to a view and a stored generated column is similar to a
materialized view (except that it is always updated automatically).
- PostgreSQL currently implements only stored generated columns.
+ <productname>PostgreSQL</productname> currently implements only stored generated columns.
</para>
<para>
--
2.25.1
From d2b9eaf168d7a168671e4f164230eaa8d4b4965d Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 7 Dec 2023 11:20:07 +0530
Subject: [PATCH 06/27] Support adding indentity column to a partitioned table
Adding an identity column to a partitioned table just means propagating
it down the partitioning hierarchy. All the partitions share the same
underlying sequence.
As for the implementation, we need to just disable the code prohibiting this
case for partitioned tables. The column definition is transformed only once.
The statements related to identity sequence are executed only once before
executing any subcommands. Thus only one sequence, associated with the identity
column, is created. The transformed column definition and related commands are
copied as they are for all the partitions. Hence default expressions of all the
partition use the same sequence when rewriting the table. Also the NOT NULL
constraints required by the identity column are propagated.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 10 ++++++++--
src/test/regress/expected/identity.out | 22 ++++++++++++++++++++++
src/test/regress/sql/identity.sql | 13 +++++++++++++
3 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5719df2b76..527a106c48 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7091,11 +7091,17 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
}
/*
- * Cannot add identity column if table has children, because identity does
- * not inherit. (Adding column and identity separately will work.)
+ * Regular inheritance children are independent enough not to inherit the
+ * identity column from parent hence can not recursively add identity
+ * column if the table has inheritance children.
+ *
+ * Partitions, on the other hand, are integral part of a partitioned table
+ * and inherit indetity column. Hence propagate identity column down the
+ * partition hierarchy.
*/
if (colDef->identity &&
recurse &&
+ rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE &&
find_inheritance_children(myrelid, NoLock) != NIL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 222b3684da..93d9a60b03 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -565,6 +565,28 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
pitest1_p2 | 08-04-2016 | from pitest1 | 4
(5 rows)
+-- add identity column
+CREATE TABLE pitest2 (f1 date NOT NULL, f2 text) PARTITION BY RANGE (f1);
+CREATE TABLE pitest2_p1 PARTITION OF pitest2 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+CREATE TABLE pitest2_p2 PARTITION OF pitest2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
+INSERT into pitest2(f1, f2) VALUES ('2016-07-2', 'from pitest2');
+INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-2', 'from pitest2');
+ALTER TABLE pitest2 ADD COLUMN f3 int GENERATED ALWAYS AS IDENTITY;
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest2_p1');
+INSERT INTO pitest2_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest2_p2');
+INSERT into pitest2(f1, f2) VALUES ('2016-07-4', 'from pitest2');
+INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-4', 'from pitest2');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+----
+ pitest2_p1 | 07-02-2016 | from pitest2 | 1
+ pitest2_p1 | 07-03-2016 | from pitest2_p1 | 3
+ pitest2_p1 | 07-04-2016 | from pitest2 | 5
+ pitest2_p2 | 08-02-2016 | from pitest2 | 2
+ pitest2_p2 | 08-03-2016 | from pitest2_p2 | 4
+ pitest2_p2 | 08-04-2016 | from pitest2 | 6
+(6 rows)
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index be29cbf119..ba655002c0 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -349,6 +349,19 @@ INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest1_p2');
INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
+-- add identity column
+CREATE TABLE pitest2 (f1 date NOT NULL, f2 text) PARTITION BY RANGE (f1);
+CREATE TABLE pitest2_p1 PARTITION OF pitest2 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+CREATE TABLE pitest2_p2 PARTITION OF pitest2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
+INSERT into pitest2(f1, f2) VALUES ('2016-07-2', 'from pitest2');
+INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-2', 'from pitest2');
+ALTER TABLE pitest2 ADD COLUMN f3 int GENERATED ALWAYS AS IDENTITY;
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest2_p1');
+INSERT INTO pitest2_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest2_p2');
+INSERT into pitest2(f1, f2) VALUES ('2016-07-4', 'from pitest2');
+INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-4', 'from pitest2');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
--
2.25.1
From 106fee38ca410855098ce3acb2d8d9092da13b64 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 11 Dec 2023 16:41:09 +0530
Subject: [PATCH 07/27] Adding identity to partitioned table adds it to all
partitions
... recursively. Additionally do not allow adding identity to only
partitioned table or a partition
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 47 +++++++++++++++++++++++---
src/test/regress/expected/identity.out | 30 ++++++++++++++++
src/test/regress/sql/identity.sql | 20 +++++++++++
3 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 527a106c48..5ae0739377 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -452,7 +452,8 @@ static ObjectAddress ATExecColumnDefault(Relation rel, const char *colName,
static ObjectAddress ATExecCookedColumnDefault(Relation rel, AttrNumber attnum,
Node *newDefault);
static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
- Node *def, LOCKMODE lockmode);
+ Node *def, LOCKMODE lockmode,
+ bool recurse, bool recursing);
static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName,
Node *def, LOCKMODE lockmode);
static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode);
@@ -4825,7 +4826,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddIdentity:
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
- /* This command never recurses */
+ /* Set up recursion for phase 2; no other prep needed */
+ if (recurse)
+ cmd->recurse = true;
pass = AT_PASS_ADD_OTHERCONSTR;
break;
case AT_SetIdentity:
@@ -5224,7 +5227,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
cur_pass, context);
Assert(cmd != NULL);
- address = ATExecAddIdentity(rel, cmd->name, cmd->def, lockmode);
+ address = ATExecAddIdentity(rel, cmd->name, cmd->def, lockmode,
+ cmd->recurse, false);
break;
case AT_SetIdentity:
cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
@@ -8105,7 +8109,7 @@ ATExecCookedColumnDefault(Relation rel, AttrNumber attnum,
*/
static ObjectAddress
ATExecAddIdentity(Relation rel, const char *colName,
- Node *def, LOCKMODE lockmode)
+ Node *def, LOCKMODE lockmode, bool recurse, bool recursing)
{
Relation attrelation;
HeapTuple tuple;
@@ -8113,6 +8117,19 @@ ATExecAddIdentity(Relation rel, const char *colName,
AttrNumber attnum;
ObjectAddress address;
ColumnDef *cdef = castNode(ColumnDef, def);
+ bool ispartitioned;
+
+ ispartitioned = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
+ if (ispartitioned && !recurse)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot add identity to a column of only the partitioned table"),
+ errhint("Do not specify the ONLY keyword.")));
+
+ if (rel->rd_rel->relispartition && !recursing)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot add identity to a column of a partition"));
attrelation = table_open(AttributeRelationId, RowExclusiveLock);
@@ -8167,6 +8184,28 @@ ATExecAddIdentity(Relation rel, const char *colName,
table_close(attrelation, RowExclusiveLock);
+ /*
+ * Recurse to propagate the identity column to partitions. Identity is not
+ * inherited in regular inheritance children.
+ */
+ if (recurse && ispartitioned)
+ {
+ List *children;
+ ListCell *lc;
+
+ children = find_inheritance_children(RelationGetRelid(rel),
+ lockmode);
+
+ foreach(lc, children)
+ {
+ Relation childrel;
+
+ childrel = table_open(lfirst_oid(lc), NoLock);
+ ATExecAddIdentity(childrel, colName, def, lockmode, recurse, true);
+ table_close(childrel, NoLock);
+ }
+ }
+
return address;
}
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 93d9a60b03..9de35470af 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -587,6 +587,36 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
pitest2_p2 | 08-04-2016 | from pitest2 | 6
(6 rows)
+-- changing a regular column to identity column in a partitioned table
+CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
+CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+INSERT into pitest3 VALUES ('2016-07-2', 'from pitest3', 1);
+INSERT into pitest3_p1 VALUES ('2016-07-3', 'from pitest3_p1', 2);
+-- fails, changing only a partition not allowed
+ALTER TABLE pitest3_p1
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+ERROR: cannot add identity to a column of a partition
+-- fails, changing only the partitioned table not allowed
+ALTER TABLE ONLY pitest3
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+ERROR: constraint must be added to child tables too
+HINT: Do not specify the ONLY keyword.
+ALTER TABLE pitest3
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3');
+INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+----
+ pitest3_p1 | 07-02-2016 | from pitest3 | 1
+ pitest3_p1 | 07-03-2016 | from pitest3_p1 | 2
+ pitest3_p1 | 07-04-2016 | from pitest3 | 3
+ pitest3_p1 | 07-05-2016 | from pitest3_p1 | 4
+(4 rows)
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index ba655002c0..9d1effa059 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -362,6 +362,26 @@ INSERT into pitest2(f1, f2) VALUES ('2016-07-4', 'from pitest2');
INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-4', 'from pitest2');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+-- changing a regular column to identity column in a partitioned table
+CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
+CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+INSERT into pitest3 VALUES ('2016-07-2', 'from pitest3', 1);
+INSERT into pitest3_p1 VALUES ('2016-07-3', 'from pitest3_p1', 2);
+-- fails, changing only a partition not allowed
+ALTER TABLE pitest3_p1
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+-- fails, changing only the partitioned table not allowed
+ALTER TABLE ONLY pitest3
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+ALTER TABLE pitest3
+ ALTER COLUMN f3 SET NOT NULL,
+ ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY (START WITH 3);
+INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3');
+INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1');
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
--
2.25.1
From 06a04b4ce767e2baaebaf112fe82b18d1be5d9a2 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 3 Jan 2024 10:00:22 +0530
Subject: [PATCH 09/27] Changing Identity column of a partitioned table
The change is propagated to all the partitions. Changing the column only
in a partitioned table or a partition is not allowed; the change needs
to be applied to the whole partition hierarchy.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 48 +++++++++++++++++++++++---
src/test/regress/expected/identity.out | 28 +++++++++++++++
src/test/regress/sql/identity.sql | 11 ++++++
3 files changed, 83 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f0c1a85031..67b38c5101 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -455,7 +455,8 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
Node *def, LOCKMODE lockmode,
bool recurse, bool recursing);
static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName,
- Node *def, LOCKMODE lockmode);
+ Node *def, LOCKMODE lockmode,
+ bool recurse, bool recursing);
static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName,
bool missing_ok, LOCKMODE lockmode,
bool recurse, bool recursing);
@@ -4835,7 +4836,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_SetIdentity:
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
- /* This command never recurses */
+ /* Set up recursion for phase 2; no other prep needed */
+ if (recurse)
+ cmd->recurse = true;
/* This should run after AddIdentity, so do it in MISC pass */
pass = AT_PASS_MISC;
break;
@@ -5238,7 +5241,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
cur_pass, context);
Assert(cmd != NULL);
- address = ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode);
+ address = ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode,
+ cmd->recurse, false);
break;
case AT_DropIdentity:
address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok,
@@ -8220,7 +8224,8 @@ ATExecAddIdentity(Relation rel, const char *colName,
* Return the address of the affected column.
*/
static ObjectAddress
-ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmode)
+ATExecSetIdentity(Relation rel, const char *colName, Node *def,
+ LOCKMODE lockmode, bool recurse, bool recursing)
{
ListCell *option;
DefElem *generatedEl = NULL;
@@ -8229,6 +8234,19 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmod
AttrNumber attnum;
Relation attrelation;
ObjectAddress address;
+ bool ispartitioned;
+
+ ispartitioned = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
+ if (ispartitioned && !recurse)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot change identity column of only the partitioned table"),
+ errhint("Do not specify the ONLY keyword.")));
+
+ if (rel->rd_rel->relispartition && !recursing)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot change identity column of a partition"));
foreach(option, castNode(List, def))
{
@@ -8293,6 +8311,28 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmod
heap_freetuple(tuple);
table_close(attrelation, RowExclusiveLock);
+ /*
+ * Recurse to propagate the identity change to partitions. Identity is not
+ * inherited in regular inheritance children.
+ */
+ if (generatedEl && recurse && ispartitioned)
+ {
+ List *children;
+ ListCell *lc;
+
+ children = find_inheritance_children(RelationGetRelid(rel),
+ lockmode);
+
+ foreach(lc, children)
+ {
+ Relation childrel;
+
+ childrel = table_open(lfirst_oid(lc), NoLock);
+ ATExecSetIdentity(childrel, colName, def, lockmode, recurse, true);
+ table_close(childrel, NoLock);
+ }
+ }
+
return address;
}
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index d2860b4347..e185e0d4a1 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -587,6 +587,34 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
pitest2_p2 | 08-04-2016 | from pitest2 | 6
(6 rows)
+-- SET identity column
+ALTER TABLE pitest2_p1 ALTER COLUMN f3 SET GENERATED BY DEFAULT; -- fails
+ERROR: cannot change identity column of a partition
+ALTER TABLE pitest2_p1 ALTER COLUMN f3 SET INCREMENT BY 2; -- fails
+ERROR: cannot change identity column of a partition
+ALTER TABLE ONLY pitest2 ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT BY 2 SET START WITH 1000 RESTART; -- fails
+ERROR: cannot change identity column of only the partitioned table
+HINT: Do not specify the ONLY keyword.
+ALTER TABLE pitest2 ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT BY 2 SET START WITH 1000 RESTART;
+INSERT into pitest2(f1, f2, f3) VALUES ('2016-07-5', 'from pitest2', 200);
+INSERT INTO pitest2(f1, f2) VALUES ('2016-08-5', 'from pitest2');
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-6', 'from pitest2_p1');
+INSERT INTO pitest2_p2 (f1, f2, f3) VALUES ('2016-08-6', 'from pitest2_p2', 300);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+------
+ pitest2_p1 | 07-02-2016 | from pitest2 | 1
+ pitest2_p1 | 07-03-2016 | from pitest2_p1 | 3
+ pitest2_p1 | 07-04-2016 | from pitest2 | 5
+ pitest2_p1 | 07-05-2016 | from pitest2 | 200
+ pitest2_p1 | 07-06-2016 | from pitest2_p1 | 1002
+ pitest2_p2 | 08-02-2016 | from pitest2 | 2
+ pitest2_p2 | 08-03-2016 | from pitest2_p2 | 4
+ pitest2_p2 | 08-04-2016 | from pitest2 | 6
+ pitest2_p2 | 08-05-2016 | from pitest2 | 1000
+ pitest2_p2 | 08-06-2016 | from pitest2_p2 | 300
+(10 rows)
+
-- changing a regular column to identity column in a partitioned table
CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 62323f9cbd..a1862df1d8 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -362,6 +362,17 @@ INSERT into pitest2(f1, f2) VALUES ('2016-07-4', 'from pitest2');
INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-4', 'from pitest2');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+-- SET identity column
+ALTER TABLE pitest2_p1 ALTER COLUMN f3 SET GENERATED BY DEFAULT; -- fails
+ALTER TABLE pitest2_p1 ALTER COLUMN f3 SET INCREMENT BY 2; -- fails
+ALTER TABLE ONLY pitest2 ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT BY 2 SET START WITH 1000 RESTART; -- fails
+ALTER TABLE pitest2 ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT BY 2 SET START WITH 1000 RESTART;
+INSERT into pitest2(f1, f2, f3) VALUES ('2016-07-5', 'from pitest2', 200);
+INSERT INTO pitest2(f1, f2) VALUES ('2016-08-5', 'from pitest2');
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-6', 'from pitest2_p1');
+INSERT INTO pitest2_p2 (f1, f2, f3) VALUES ('2016-08-6', 'from pitest2_p2', 300);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+
-- changing a regular column to identity column in a partitioned table
CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
--
2.25.1
From 6f5276cef8814300057a370778bb9b8ce6ebde23 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 18 Dec 2023 16:39:30 +0530
Subject: [PATCH 08/27] DROP IDENTITY on partitioned table recurses to its
partitions
Since identity property is inherited by partition tables, dropping
identity of a column in partitioned table should result in dropping it
from all the partitions.
I tried to implement the DROP SEQUENCE as a separate step to be executed
at the end of ALTER TABLE command. But that does not work since the
sequence has a dependency on the column (whose identity is being
dropped) and not on the identity property itself. There is no step/ALTER
TABLE command to drop that dependecy.
Dropping identity property from a column of a partition table is not
allwoed.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 73 +++++++++++++++++++++-----
src/test/regress/expected/identity.out | 26 +++++++++
src/test/regress/sql/identity.sql | 10 ++++
3 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5ae0739377..f0c1a85031 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -456,7 +456,9 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
bool recurse, bool recursing);
static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName,
Node *def, LOCKMODE lockmode);
-static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode);
+static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName,
+ bool missing_ok, LOCKMODE lockmode,
+ bool recurse, bool recursing);
static void ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode);
static ObjectAddress ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode);
static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum,
@@ -4839,7 +4841,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_DropIdentity:
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
- /* This command never recurses */
+ /* Set up recursion for phase 2; no other prep needed */
+ if (recurse)
+ cmd->recurse = true;
pass = AT_PASS_DROP;
break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
@@ -5237,7 +5241,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
address = ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode);
break;
case AT_DropIdentity:
- address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok, lockmode);
+ address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok,
+ lockmode, cmd->recurse, false);
break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
address = ATExecDropNotNull(rel, cmd->name, cmd->recurse, lockmode);
@@ -8297,7 +8302,9 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmod
* Return the address of the affected column.
*/
static ObjectAddress
-ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode)
+ATExecDropIdentity(Relation rel, const char *colName,
+ bool missing_ok, LOCKMODE lockmode,
+ bool recurse, bool recursing)
{
HeapTuple tuple;
Form_pg_attribute attTup;
@@ -8306,6 +8313,19 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE
ObjectAddress address;
Oid seqid;
ObjectAddress seqaddress;
+ bool ispartitioned;
+
+ ispartitioned = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
+ if (ispartitioned && !recurse)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot drop identity from a column of only the partitioned table"),
+ errhint("Do not specify the ONLY keyword.")));
+
+ if (rel->rd_rel->relispartition && !recursing)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot drop identity from a column of a partition"));
attrelation = table_open(AttributeRelationId, RowExclusiveLock);
tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
@@ -8354,15 +8374,42 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE
table_close(attrelation, RowExclusiveLock);
- /* drop the internal sequence */
- seqid = getIdentitySequence(RelationGetRelid(rel), attnum, false);
- deleteDependencyRecordsForClass(RelationRelationId, seqid,
- RelationRelationId, DEPENDENCY_INTERNAL);
- CommandCounterIncrement();
- seqaddress.classId = RelationRelationId;
- seqaddress.objectId = seqid;
- seqaddress.objectSubId = 0;
- performDeletion(&seqaddress, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+ /*
+ * Recurse to drop the identity from column in partitions. Identity is not
+ * inherited in regular inheritance children so ignore them.
+ */
+ if (recurse && ispartitioned)
+ {
+ List *children;
+ ListCell *lc;
+
+ children = find_inheritance_children(RelationGetRelid(rel),
+ lockmode);
+
+ foreach(lc, children)
+ {
+ Relation childrel;
+
+ childrel = table_open(lfirst_oid(lc), NoLock);
+ ATExecDropIdentity(childrel, colName, false, lockmode, recurse,
+ true);
+ table_close(childrel, NoLock);
+ }
+ }
+
+ if (!recursing)
+ {
+ /* drop the internal sequence */
+ seqid = getIdentitySequence(RelationGetRelid(rel), attnum, false);
+ deleteDependencyRecordsForClass(RelationRelationId, seqid,
+ RelationRelationId,
+ DEPENDENCY_INTERNAL);
+ CommandCounterIncrement();
+ seqaddress.classId = RelationRelationId;
+ seqaddress.objectId = seqid;
+ seqaddress.objectSubId = 0;
+ performDeletion(&seqaddress, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+ }
return address;
}
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 9de35470af..d2860b4347 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -617,6 +617,32 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
pitest3_p1 | 07-05-2016 | from pitest3_p1 | 4
(4 rows)
+-- changing an identity column to a non-identity column in a partitioned table
+ALTER TABLE pitest3_p1 ALTER COLUMN f3 DROP IDENTITY; -- fails
+ERROR: cannot drop identity from a column of a partition
+ALTER TABLE ONLY pitest3 ALTER COLUMN f3 DROP IDENTITY; -- fails
+ERROR: cannot drop identity from a column of only the partitioned table
+HINT: Do not specify the ONLY keyword.
+ALTER TABLE pitest3 ALTER COLUMN f3 DROP IDENTITY;
+INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3'); -- fails
+ERROR: null value in column "f3" of relation "pitest3_p1" violates not-null constraint
+DETAIL: Failing row contains (07-04-2016, from pitest3, null).
+INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1'); -- fails
+ERROR: null value in column "f3" of relation "pitest3_p1" violates not-null constraint
+DETAIL: Failing row contains (07-05-2016, from pitest3_p1, null).
+INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5);
+INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+----
+ pitest3_p1 | 07-02-2016 | from pitest3 | 1
+ pitest3_p1 | 07-03-2016 | from pitest3_p1 | 2
+ pitest3_p1 | 07-04-2016 | from pitest3 | 3
+ pitest3_p1 | 07-05-2016 | from pitest3_p1 | 4
+ pitest3_p1 | 07-06-2016 | from pitest3 | 5
+ pitest3_p1 | 07-07-2016 | from pitest3_p1 | 6
+(6 rows)
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 9d1effa059..62323f9cbd 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -382,6 +382,16 @@ INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3');
INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1');
SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+-- changing an identity column to a non-identity column in a partitioned table
+ALTER TABLE pitest3_p1 ALTER COLUMN f3 DROP IDENTITY; -- fails
+ALTER TABLE ONLY pitest3 ALTER COLUMN f3 DROP IDENTITY; -- fails
+ALTER TABLE pitest3 ALTER COLUMN f3 DROP IDENTITY;
+INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3'); -- fails
+INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1'); -- fails
+INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5);
+INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+
-- partition with identity column of its own is not allowed
CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
CREATE TABLE itest_child PARTITION OF itest_parent (
--
2.25.1
From 1a5517a57ac5f3dd083b4814d403e35f26a542f5 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Tue, 19 Dec 2023 11:38:48 +0530
Subject: [PATCH 10/27] Drop identity property when detaching partition
A partition's identity column shares the identity space (i.e. underlying
sequence) as the corresponding column of the partitioned table. If a
partition is detached it can longer share the identity space as before.
Hence the identity columns of the partition being detached loose their
identity property.
When identity of a column of a regular table is dropped it retains the
NOT NULL constarint that came with the identity property. Similarly the
columns of the partition being detached retain the NOT NULL constraints
that came with identity property, even though the identity property
itself is lost.
Also note that the sequence associated with the identity property is
linked to the partitioned table (and not the partition being detached).
That sequence is not dropped as part of detach operation.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 13 +++++++++++
src/test/regress/expected/identity.out | 30 ++++++++++++++++++++++++++
src/test/regress/sql/identity.sql | 10 +++++++++
3 files changed, 53 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 67b38c5101..19badb3fba 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19498,6 +19498,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
HeapTuple tuple,
newtuple;
Relation trigrel = NULL;
+ int attno;
if (concurrent)
{
@@ -19663,6 +19664,18 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
heap_freetuple(newtuple);
table_close(classRel, RowExclusiveLock);
+ /*
+ * Drop identity property from all identity columns of partition.
+ */
+ for (attno = 0; attno < RelationGetNumberOfAttributes(partRel); attno++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(partRel->rd_att, attno);
+
+ if (!attr->attisdropped && attr->attidentity)
+ ATExecDropIdentity(partRel, NameStr(attr->attname), false,
+ AccessExclusiveLock, true, true);
+ }
+
if (OidIsValid(defaultPartOid))
{
/*
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index e185e0d4a1..501fcc22c6 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -615,6 +615,36 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
pitest2_p2 | 08-06-2016 | from pitest2_p2 | 300
(10 rows)
+-- detaching a partition removes identity property
+ALTER TABLE pitest2 DETACH PARTITION pitest2_p1;
+INSERT into pitest2(f1, f2) VALUES ('2016-08-7', 'from pitest2');
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-7', 'from pitest2_p1'); -- error
+ERROR: null value in column "f3" of relation "pitest2_p1" violates not-null constraint
+DETAIL: Failing row contains (07-07-2016, from pitest2_p1, null).
+INSERT into pitest2_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest2_p1', 2000);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+------
+ pitest2_p2 | 08-02-2016 | from pitest2 | 2
+ pitest2_p2 | 08-03-2016 | from pitest2_p2 | 4
+ pitest2_p2 | 08-04-2016 | from pitest2 | 6
+ pitest2_p2 | 08-05-2016 | from pitest2 | 1000
+ pitest2_p2 | 08-06-2016 | from pitest2_p2 | 300
+ pitest2_p2 | 08-07-2016 | from pitest2 | 1004
+(6 rows)
+
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2_p1;
+ tableoid | f1 | f2 | f3
+------------+------------+-----------------+------
+ pitest2_p1 | 07-02-2016 | from pitest2 | 1
+ pitest2_p1 | 07-03-2016 | from pitest2_p1 | 3
+ pitest2_p1 | 07-04-2016 | from pitest2 | 5
+ pitest2_p1 | 07-05-2016 | from pitest2 | 200
+ pitest2_p1 | 07-06-2016 | from pitest2_p1 | 1002
+ pitest2_p1 | 07-07-2016 | from pitest2_p1 | 2000
+(6 rows)
+
+DROP TABLE pitest2_p1;
-- changing a regular column to identity column in a partitioned table
CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index a1862df1d8..0d7ecfb3e0 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -373,6 +373,16 @@ INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-6', 'from pitest2_p1');
INSERT INTO pitest2_p2 (f1, f2, f3) VALUES ('2016-08-6', 'from pitest2_p2', 300);
SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+-- detaching a partition removes identity property
+ALTER TABLE pitest2 DETACH PARTITION pitest2_p1;
+INSERT into pitest2(f1, f2) VALUES ('2016-08-7', 'from pitest2');
+INSERT into pitest2_p1 (f1, f2) VALUES ('2016-07-7', 'from pitest2_p1'); -- error
+INSERT into pitest2_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest2_p1', 2000);
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;
+SELECT tableoid::regclass, f1, f2, f3 FROM pitest2_p1;
+
+DROP TABLE pitest2_p1;
+
-- changing a regular column to identity column in a partitioned table
CREATE TABLE pitest3 (f1 date NOT NULL, f2 text, f3 int) PARTITION BY RANGE (f1);
CREATE TABLE pitest3_p1 PARTITION OF pitest3 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
--
2.25.1
From 3ace7b78b04395e01a973e49af1e7b8f29bbafae Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 20 Dec 2023 14:55:24 +0530
Subject: [PATCH 11/27] Partitions with their own identity columns are not
allowed
... for the reasons specified in earlier commit messages.
Ashutosh Bapat
---
src/backend/commands/tablecmds.c | 12 +++++++++++-
src/test/regress/expected/generated.out | 2 +-
src/test/regress/expected/identity.out | 20 +++++++++++++++-----
src/test/regress/sql/identity.sql | 19 ++++++++++++++-----
4 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 19badb3fba..f6db45522f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18793,7 +18793,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot attach temporary relation of another session as partition")));
- /* Check if there are any columns in attachrel that aren't in the parent */
+ /*
+ * Check if attachrel has any identity columns or any columns that aren't
+ * in the parent.
+ */
tupleDesc = RelationGetDescr(attachrel);
natts = tupleDesc->natts;
for (attno = 1; attno <= natts; attno++)
@@ -18805,6 +18808,13 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
if (attribute->attisdropped)
continue;
+ if (attribute->attidentity)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("table \"%s\" being attached can not contain identity column \"%s\"",
+ RelationGetRelationName(attachrel),
+ attributeName));
+
/* Try to find the column in parent (matching on column name) */
if (!SearchSysCacheExists2(ATTNAME,
ObjectIdGetDatum(RelationGetRelid(rel)),
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index a2f38d0f50..c9b0856fa2 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -753,7 +753,7 @@ ERROR: column "f3" in child table must be a generated column
DROP TABLE gtest_child3;
CREATE TABLE gtest_child3 (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS IDENTITY);
ALTER TABLE gtest_parent ATTACH PARTITION gtest_child3 FOR VALUES FROM ('2016-09-01') TO ('2016-10-01'); -- error
-ERROR: column "f3" in child table must be a generated column
+ERROR: table "gtest_child3" being attached can not contain identity column "f3"
DROP TABLE gtest_child3;
CREATE TABLE gtest_child3 (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 33) STORED);
ALTER TABLE gtest_parent ATTACH PARTITION gtest_child3 FOR VALUES FROM ('2016-09-01') TO ('2016-10-01');
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 501fcc22c6..0b6a4b160e 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -701,13 +701,23 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
pitest3_p1 | 07-07-2016 | from pitest3_p1 | 6
(6 rows)
--- partition with identity column of its own is not allowed
-CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
-CREATE TABLE itest_child PARTITION OF itest_parent (
+-- partitions with their own identity columns are not allowed, even if the
+-- partitioned table does not have an identity column.
+CREATE TABLE pitest1_pfail PARTITION OF pitest1 (
f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
-) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01'); -- error
+) FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
ERROR: identity columns are not supported on partitions
-DROP TABLE itest_parent;
+CREATE TABLE pitest_pfail PARTITION OF pitest3 (
+ f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
+) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+ERROR: identity columns are not supported on partitions
+CREATE TABLE pitest1_pfail (f1 date NOT NULL, f2 text, f3 bigint GENERATED ALWAYS AS IDENTITY);
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_pfail FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
+ERROR: table "pitest1_pfail" being attached can not contain identity column "f3"
+ALTER TABLE pitest3 ATTACH PARTITION pitest1_pfail FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
+ERROR: table "pitest1_pfail" being attached can not contain identity column "f3"
+DROP TABLE pitest1_pfail;
+DROP TABLE pitest3;
-- test that sequence of half-dropped serial column is properly ignored
CREATE TABLE itest14 (id serial);
ALTER TABLE itest14 ALTER id DROP DEFAULT;
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 0d7ecfb3e0..e0ce2c799b 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -413,13 +413,22 @@ INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5);
INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6);
SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
--- partition with identity column of its own is not allowed
-CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1);
-CREATE TABLE itest_child PARTITION OF itest_parent (
+-- partitions with their own identity columns are not allowed, even if the
+-- partitioned table does not have an identity column.
+CREATE TABLE pitest1_pfail PARTITION OF pitest1 (
f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
-) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01'); -- error
-DROP TABLE itest_parent;
+) FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
+CREATE TABLE pitest_pfail PARTITION OF pitest3 (
+ f3 WITH OPTIONS GENERATED ALWAYS AS IDENTITY
+) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
+
+CREATE TABLE pitest1_pfail (f1 date NOT NULL, f2 text, f3 bigint GENERATED ALWAYS AS IDENTITY);
+ALTER TABLE pitest1 ATTACH PARTITION pitest1_pfail FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
+ALTER TABLE pitest3 ATTACH PARTITION pitest1_pfail FOR VALUES FROM ('2016-11-01') TO ('2016-12-01');
+
+DROP TABLE pitest1_pfail;
+DROP TABLE pitest3;
-- test that sequence of half-dropped serial column is properly ignored
--
2.25.1
From ab37ba8f8a517c8aea8c6cb05eb3712b5da15955 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 3 Jan 2024 11:54:22 +0530
Subject: [PATCH 12/27] Test changing some properties of identity column in
partitioned table
NULL constraint, default and identity property
Ashutosh Bapat
---
src/test/regress/expected/identity.out | 15 +++++++++++++++
src/test/regress/sql/identity.sql | 10 ++++++++++
2 files changed, 25 insertions(+)
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 0b6a4b160e..f03d88035f 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -701,6 +701,21 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
pitest3_p1 | 07-07-2016 | from pitest3_p1 | 6
(6 rows)
+-- Changing NOT NULL constraint of identity columns is not allowed
+ALTER TABLE pitest1_p1 ALTER COLUMN f3 DROP NOT NULL;
+ERROR: column "f3" of relation "pitest1_p1" is an identity column
+ALTER TABLE pitest1 ALTER COLUMN f3 DROP NOT NULL;
+ERROR: column "f3" of relation "pitest1" is an identity column
+-- Identity columns have their own default
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET DEFAULT 10000;
+ERROR: column "f3" of relation "pitest1_p2" is an identity column
+ALTER TABLE pitest1 ALTER COLUMN f3 SET DEFAULT 10000;
+ERROR: column "f3" of relation "pitest1" is an identity column
+-- Adding identity to an identity column is not allowed
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 ADD GENERATED BY DEFAULT AS IDENTITY;
+ERROR: cannot add identity to a column of a partition
+ALTER TABLE pitest1 ALTER COLUMN f3 ADD GENERATED BY DEFAULT AS IDENTITY;
+ERROR: column "f3" of relation "pitest1" is already an identity column
-- partitions with their own identity columns are not allowed, even if the
-- partitioned table does not have an identity column.
CREATE TABLE pitest1_pfail PARTITION OF pitest1 (
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index e0ce2c799b..9f35214751 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -413,6 +413,16 @@ INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5);
INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6);
SELECT tableoid::regclass, f1, f2, f3 FROM pitest3;
+-- Changing NOT NULL constraint of identity columns is not allowed
+ALTER TABLE pitest1_p1 ALTER COLUMN f3 DROP NOT NULL;
+ALTER TABLE pitest1 ALTER COLUMN f3 DROP NOT NULL;
+-- Identity columns have their own default
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET DEFAULT 10000;
+ALTER TABLE pitest1 ALTER COLUMN f3 SET DEFAULT 10000;
+-- Adding identity to an identity column is not allowed
+ALTER TABLE pitest1_p2 ALTER COLUMN f3 ADD GENERATED BY DEFAULT AS IDENTITY;
+ALTER TABLE pitest1 ALTER COLUMN f3 ADD GENERATED BY DEFAULT AS IDENTITY;
+
-- partitions with their own identity columns are not allowed, even if the
-- partitioned table does not have an identity column.
CREATE TABLE pitest1_pfail PARTITION OF pitest1 (
--
2.25.1
From d5f226c0542c1524ef62fa80ae9fef9d5513e685 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Mon, 8 Jan 2024 16:22:23 +0530 Subject: [PATCH 13/27] Fix output in modules/test_ddl_deparse/ ALTER TABLE ... ALTER COLUMN ... ADD/SET/DROP IDENTITY commands need to recurse into partition hierarchy. Hence they are marked to recurse when ONLY is not specified. Change the expected output for the same. Please note that in case of regular inheritance the commands won't recurse down the inheritance tree whether or not ONLY is specified. This is the same as the earlier behaviour. Ashutosh Bapat --- src/test/modules/test_ddl_deparse/expected/alter_table.out | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/modules/test_ddl_deparse/expected/alter_table.out b/src/test/modules/test_ddl_deparse/expected/alter_table.out index ecde9d7422..b5e71af9aa 100644 --- a/src/test/modules/test_ddl_deparse/expected/alter_table.out +++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out @@ -74,14 +74,14 @@ ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type simple, tag ALTER SEQUENCE NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type ADD IDENTITY desc column a of table parent +NOTICE: subcommand: type ADD IDENTITY (and recurse) desc column a of table parent ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT; NOTICE: DDL test: type simple, tag ALTER SEQUENCE NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type SET IDENTITY desc column a of table parent +NOTICE: subcommand: type SET IDENTITY (and recurse) desc column a of table parent ALTER TABLE parent ALTER COLUMN a DROP IDENTITY; NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type DROP IDENTITY desc column a of table parent +NOTICE: subcommand: type DROP IDENTITY (and recurse) desc column a of table parent ALTER TABLE parent ALTER COLUMN a SET STATISTICS 100; NOTICE: DDL test: type alter table, tag ALTER TABLE NOTICE: subcommand: type SET STATS desc column a of table parent -- 2.25.1
From e18135cc29c074c23113f6cb907cbe83fe22e248 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Fri, 5 Jan 2024 19:28:16 +0530
Subject: [PATCH 14/27] Test dump and restore: NOT FOR FINAL COMMIT
... through the changed testcase since it dumps and restores the objects
left behind by sql/identity.sql.
The additional SQL statements here test the sanity of the restored
partitioned tables with identity column in it. I have not seen these
kind of tests for checking sanity of other objects so it's quite
possible that dump comparison performed by the test is enough. Hence not
planning to propose this for final commit.
The earlier patches do not change anything in dump restore and yet it
works. This is because the identity columns are dumped using ALTER TABLE
command, which is constructed only for the table owning the underlying
sequence. In case of partitioned tables, only the topmost partitioned
table owns the sequence and thus ALTER TABLE command adding identity to
the column is run only on the topmost table. The partitions are attached
afterwards and inherit the identity column as part of that operation.
Ashutosh Bapat
---
src/bin/pg_upgrade/t/002_pg_upgrade.pl | 33 ++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index b0470844de..26ee1fb2af 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -458,4 +458,37 @@ if ($compare_res != 0)
print "=== EOF ===\n";
}
+# Test restored partitioned table with identity
+$newnode->safe_psql('regression',
+ "INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-5', 'from pitest1'), ('2016-07-5', 'from pitest1');");
+$newnode->safe_psql('regression',
+ "INSERT INTO pitest1_p1 (f1, f2) VALUES ('2016-07-6', 'from pitest1_p1');");
+$newnode->safe_psql('regression',
+ "INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-6', 'from pitest1_p2');");
+$result = $newnode->safe_psql('regression',
+ "SELECT tableoid::regclass, f1, f2, f3 FROM pitest1");
+is($result,"pitest1_p1|2016-07-02|from pitest1|1
+pitest1_p1|2016-07-03|from pitest1_p1|2
+pitest1_p1|2016-07-05|from pitest1|6
+pitest1_p1|2016-07-06|from pitest1_p1|7
+pitest1_p2|2016-08-02|before attaching|100
+pitest1_p2|2016-08-03|from pitest1_p2|3
+pitest1_p2|2016-08-04|from pitest1|4
+pitest1_p2|2016-08-05|from pitest1|5
+pitest1_p2|2016-08-06|from pitest1_p2|8");
+$newnode->safe_psql('regression',
+ "INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-8', 'from pitest2');");
+$newnode->safe_psql('regression',
+ "INSERT INTO pitest2_p2 (f1, f2) VALUES ('2016-08-9', 'from pitest2_p2');");
+$result = $newnode->safe_psql('regression',
+ "SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;");
+is($result, "pitest2_p2|2016-08-02|from pitest2|2
+pitest2_p2|2016-08-03|from pitest2_p2|4
+pitest2_p2|2016-08-04|from pitest2|6
+pitest2_p2|2016-08-05|from pitest2|1000
+pitest2_p2|2016-08-06|from pitest2_p2|300
+pitest2_p2|2016-08-07|from pitest2|1004
+pitest2_p2|2016-08-08|from pitest2|1006
+pitest2_p2|2016-08-09|from pitest2_p2|1008");
+
done_testing();
--
2.25.1
From 304969e542940240e21698b290cc647a38f066fb Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 8 Jan 2024 15:01:06 +0530
Subject: [PATCH 15/27] Test pg_upgrade
NOT FOR FINAL COMMIT
Old clusters, with versions 10 and above, may have partitioned tables
with identity columns. The corresponding columns in partitions of such
tables will not be marked as identity columns. Test that such
partitioned tables are upgraded correctly. The columns in partitions are
also marked as identity columns and use the same underlying sequence as
the partitioned table.
Test this using following steps
1. On an old version cluster run following commands to create required
partitioned tables in "regression" database
--- sql
CREATE TABLE pitest1 (f1 date NOT NULL, f2 text, f3 bigint generated always as identity) PARTITION BY RANGE (f1);
CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
INSERT into pitest1_p1 (f1, f2, f3) VALUES ('2016-07-3', 'from pitest1_p1', nextval('pitest1_f3_seq'));
CREATE TABLE pitest1_p2 (f1 date NOT NULL, f2 text, f3 bigint);
INSERT INTO pitest1_p2 VALUES ('2016-08-2', 'before attaching', 100);
ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET NOT NULL;
ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
INSERT INTO pitest1_p2 (f1, f2, f3) VALUES ('2016-08-3', 'from pitest1_p2', nextval('pitest1_f3_seq'));
INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
--- sql
2. Take dump using pg_dumpall and save it in a file
3. Set environment variables olddump to the path of this file and oldinstall to
the binaries of old version.
4. Run modified 002_pg_upgrade.
---
src/bin/pg_upgrade/t/002_pg_upgrade.pl | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 26ee1fb2af..9b63a7b9dd 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -241,8 +241,9 @@ if (defined($ENV{oldinstall}))
my %dbnames;
do { $dbnames{$_} = 1; }
foreach split /\s+/s, $dbnames;
- my $adjust_cmds =
- adjust_database_contents($oldnode->pg_version, %dbnames);
+ # Testing a custom dump that doesn't have regression objects
+ my $adjust_cmds = ();
+ # adjust_database_contents($oldnode->pg_version, %dbnames);
foreach my $updb (keys %$adjust_cmds)
{
@@ -476,19 +477,5 @@ pitest1_p2|2016-08-03|from pitest1_p2|3
pitest1_p2|2016-08-04|from pitest1|4
pitest1_p2|2016-08-05|from pitest1|5
pitest1_p2|2016-08-06|from pitest1_p2|8");
-$newnode->safe_psql('regression',
- "INSERT INTO pitest2 (f1, f2) VALUES ('2016-08-8', 'from pitest2');");
-$newnode->safe_psql('regression',
- "INSERT INTO pitest2_p2 (f1, f2) VALUES ('2016-08-9', 'from pitest2_p2');");
-$result = $newnode->safe_psql('regression',
- "SELECT tableoid::regclass, f1, f2, f3 FROM pitest2;");
-is($result, "pitest2_p2|2016-08-02|from pitest2|2
-pitest2_p2|2016-08-03|from pitest2_p2|4
-pitest2_p2|2016-08-04|from pitest2|6
-pitest2_p2|2016-08-05|from pitest2|1000
-pitest2_p2|2016-08-06|from pitest2_p2|300
-pitest2_p2|2016-08-07|from pitest2|1004
-pitest2_p2|2016-08-08|from pitest2|1006
-pitest2_p2|2016-08-09|from pitest2_p2|1008");
done_testing();
--
2.25.1
pg_dumpall_14.out
Description: Binary data
