Skip to content

Commit 5091995

Browse files
peterepetere
committed
Fix assorted bugs related to identity column in partitioned tables
When changing the data type of a column of a partitioned table, craft the ALTER SEQUENCE command only once. Partitions do not have identity sequences of their own and thus do not need a ALTER SEQUENCE command for each partition. Fix getIdentitySequence() to fetch the identity sequence associated with the top-level partitioned table when a Relation of a partition is passed to it. While doing so, translate the attribute number of the partition into the attribute number of the partitioned table. Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Discussion: https://www.postgresql.org/message-id/3b8a9dc1-bbc7-0ef5-6863-c432afac7d59@gmail.com
1 parent 832c4f6 commit 5091995

File tree

7 files changed

+101
-48
lines changed

7 files changed

+101
-48
lines changed

‎src/backend/catalog/pg_depend.c

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
#include "catalog/pg_constraint.h"
2424
#include "catalog/pg_depend.h"
2525
#include "catalog/pg_extension.h"
26+
#include "catalog/partition.h"
2627
#include "commands/extension.h"
2728
#include "miscadmin.h"
2829
#include "utils/fmgroids.h"
2930
#include "utils/lsyscache.h"
31+
#include "utils/syscache.h"
3032
#include "utils/rel.h"
3133

3234

@@ -941,10 +943,35 @@ getOwnedSequences(Oid relid)
941943
* Get owned identity sequence, error if not exactly one.
942944
*/
943945
Oid
944-
getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok)
946+
getIdentitySequence(Relation rel, AttrNumber attnum, bool missing_ok)
945947
{
946-
List *seqlist = getOwnedSequences_internal(relid, attnum, DEPENDENCY_INTERNAL);
948+
Oid relid;
949+
List *seqlist;
947950

951+
/*
952+
* The identity sequence is associated with the topmost partitioned table,
953+
* which might have column order different than the given partition.
954+
*/
955+
if (RelationGetForm(rel)->relispartition)
956+
{
957+
List *ancestors =
958+
get_partition_ancestors(RelationGetRelid(rel));
959+
HeapTuple ctup = SearchSysCacheAttNum(RelationGetRelid(rel), attnum);
960+
const char *attname = NameStr(((Form_pg_attribute) GETSTRUCT(ctup))->attname);
961+
HeapTuple ptup;
962+
963+
relid = llast_oid(ancestors);
964+
ptup = SearchSysCacheAttName(relid, attname);
965+
attnum = ((Form_pg_attribute) GETSTRUCT(ptup))->attnum;
966+
967+
ReleaseSysCache(ctup);
968+
ReleaseSysCache(ptup);
969+
list_free(ancestors);
970+
}
971+
else
972+
relid = RelationGetRelid(rel);
973+
974+
seqlist = getOwnedSequences_internal(relid, attnum, DEPENDENCY_INTERNAL);
948975
if (list_length(seqlist) > 1)
949976
elog(ERROR, "more than one owned sequence found");
950977
else if (seqlist == NIL)

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8535,7 +8535,7 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE
85358535
if (!recursing)
85368536
{
85378537
/* drop the internal sequence */
8538-
seqid = getIdentitySequence(RelationGetRelid(rel), attnum, false);
8538+
seqid = getIdentitySequence(rel, attnum, false);
85398539
deleteDependencyRecordsForClass(RelationRelationId, seqid,
85408540
RelationRelationId, DEPENDENCY_INTERNAL);
85418541
CommandCounterIncrement();

‎src/backend/parser/parse_utilcmd.c

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
11361136
* find sequence owned by old column; extract sequence parameters;
11371137
* build new create sequence command
11381138
*/
1139-
seq_relid = getIdentitySequence(RelationGetRelid(relation), attribute->attnum, false);
1139+
seq_relid = getIdentitySequence(relation, attribute->attnum, false);
11401140
seq_options = sequence_options(seq_relid);
11411141
generateSerialExtraStmts(cxt, def,
11421142
InvalidOid, seq_options,
@@ -3716,28 +3716,36 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
37163716

37173717
/*
37183718
* For identity column, create ALTER SEQUENCE command to
3719-
* change the data type of the sequence.
3719+
* change the data type of the sequence. Identity sequence
3720+
* is associated with the top level partitioned table.
3721+
* Hence ignore partitions.
37203722
*/
3721-
attnum = get_attnum(relid, cmd->name);
3722-
if (attnum == InvalidAttrNumber)
3723-
ereport(ERROR,
3724-
(errcode(ERRCODE_UNDEFINED_COLUMN),
3725-
errmsg("column \"%s\" of relation \"%s\" does not exist",
3726-
cmd->name, RelationGetRelationName(rel))));
3727-
3728-
if (attnum > 0 &&
3729-
TupleDescAttr(tupdesc, attnum - 1)->attidentity)
3723+
if (!RelationGetForm(rel)->relispartition)
37303724
{
3731-
Oid seq_relid = getIdentitySequence(relid, attnum, false);
3732-
Oid typeOid = typenameTypeId(pstate, def->typeName);
3733-
AlterSeqStmt *altseqstmt = makeNode(AlterSeqStmt);
3734-
3735-
altseqstmt->sequence = makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
3736-
get_rel_name(seq_relid),
3737-
-1);
3738-
altseqstmt->options = list_make1(makeDefElem("as", (Node *) makeTypeNameFromOid(typeOid, -1), -1));
3739-
altseqstmt->for_identity = true;
3740-
cxt.blist = lappend(cxt.blist, altseqstmt);
3725+
attnum = get_attnum(relid, cmd->name);
3726+
if (attnum == InvalidAttrNumber)
3727+
ereport(ERROR,
3728+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3729+
errmsg("column \"%s\" of relation \"%s\" does not exist",
3730+
cmd->name, RelationGetRelationName(rel))));
3731+
3732+
if (attnum > 0 &&
3733+
TupleDescAttr(tupdesc, attnum - 1)->attidentity)
3734+
{
3735+
Oid seq_relid = getIdentitySequence(rel, attnum, false);
3736+
Oid typeOid = typenameTypeId(pstate, def->typeName);
3737+
AlterSeqStmt *altseqstmt = makeNode(AlterSeqStmt);
3738+
3739+
altseqstmt->sequence
3740+
= makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
3741+
get_rel_name(seq_relid),
3742+
-1);
3743+
altseqstmt->options = list_make1(makeDefElem("as",
3744+
(Node *) makeTypeNameFromOid(typeOid, -1),
3745+
-1));
3746+
altseqstmt->for_identity = true;
3747+
cxt.blist = lappend(cxt.blist, altseqstmt);
3748+
}
37413749
}
37423750

37433751
newcmds = lappend(newcmds, cmd);
@@ -3803,7 +3811,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
38033811
errmsg("column \"%s\" of relation \"%s\" does not exist",
38043812
cmd->name, RelationGetRelationName(rel))));
38053813

3806-
seq_relid = getIdentitySequence(relid, attnum, true);
3814+
seq_relid = getIdentitySequence(rel, attnum, true);
38073815

38083816
if (seq_relid)
38093817
{

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include "access/sysattr.h"
2525
#include "access/table.h"
2626
#include "catalog/dependency.h"
27-
#include "catalog/partition.h"
2827
#include "commands/trigger.h"
2928
#include "executor/executor.h"
3029
#include "foreign/fdwapi.h"
@@ -1233,24 +1232,8 @@ build_column_default(Relation rel, int attrno)
12331232
if (att_tup->attidentity)
12341233
{
12351234
NextValueExpr *nve = makeNode(NextValueExpr);
1236-
Oid reloid;
12371235

1238-
/*
1239-
* The identity sequence is associated with the topmost partitioned
1240-
* table.
1241-
*/
1242-
if (rel->rd_rel->relispartition)
1243-
{
1244-
List *ancestors =
1245-
get_partition_ancestors(RelationGetRelid(rel));
1246-
1247-
reloid = llast_oid(ancestors);
1248-
list_free(ancestors);
1249-
}
1250-
else
1251-
reloid = RelationGetRelid(rel);
1252-
1253-
nve->seqid = getIdentitySequence(reloid, attrno, false);
1236+
nve->seqid = getIdentitySequence(rel, attrno, false);
12541237
nve->typeId = att_tup->atttypid;
12551238

12561239
return (Node *) nve;

‎src/include/catalog/dependency.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ extern List *getAutoExtensionsOfObject(Oid classId, Oid objectId);
177177

178178
extern bool sequenceIsOwned(Oid seqId, char deptype, Oid *tableId, int32 *colId);
179179
extern List *getOwnedSequences(Oid relid);
180-
extern Oid getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok);
180+
extern Oid getIdentitySequence(Relation rel, AttrNumber attnum, bool missing_ok);
181181

182182
extern Oid get_index_constraint(Oid indexId);
183183

‎src/test/regress/expected/identity.out

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,14 +619,17 @@ CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO (
619619
INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
620620
INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
621621
-- attached partition
622-
CREATE TABLE pitest1_p2 (f1 date NOT NULL, f2 text, f3 bigint);
623-
INSERT INTO pitest1_p2 VALUES ('2016-08-2', 'before attaching', 100);
622+
CREATE TABLE pitest1_p2 (f3 bigint, f2 text, f1 date NOT NULL);
623+
INSERT INTO pitest1_p2 (f1, f2, f3) VALUES ('2016-08-2', 'before attaching', 100);
624624
ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01'); -- requires NOT NULL constraint
625625
ERROR: column "f3" in child table must be marked NOT NULL
626626
ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET NOT NULL;
627627
ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
628628
INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest1_p2');
629629
INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
630+
-- LIKE INCLUDING on partition
631+
CREATE TABLE pitest1_p1_like (LIKE pitest1_p1 INCLUDING IDENTITY);
632+
INSERT into pitest1_p1_like(f1, f2) VALUES ('2016-07-2', 'from pitest1_p1_like');
630633
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
631634
tableoid | f1 | f2 | f3
632635
------------+------------+------------------+-----
@@ -637,6 +640,31 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
637640
pitest1_p2 | 08-04-2016 | from pitest1 | 4
638641
(5 rows)
639642

643+
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1_p1_like;
644+
tableoid | f1 | f2 | f3
645+
-----------------+------------+----------------------+----
646+
pitest1_p1_like | 07-02-2016 | from pitest1_p1_like | 1
647+
(1 row)
648+
649+
ALTER TABLE pitest1 ALTER COLUMN f3 SET DATA TYPE bigint;
650+
SELECT tableoid::regclass, f1, f2, f3, pg_typeof(f3) FROM pitest1;
651+
tableoid | f1 | f2 | f3 | pg_typeof
652+
------------+------------+------------------+-----+-----------
653+
pitest1_p1 | 07-02-2016 | from pitest1 | 1 | bigint
654+
pitest1_p1 | 07-03-2016 | from pitest1_p1 | 2 | bigint
655+
pitest1_p2 | 08-02-2016 | before attaching | 100 | bigint
656+
pitest1_p2 | 08-03-2016 | from pitest1_p2 | 3 | bigint
657+
pitest1_p2 | 08-04-2016 | from pitest1 | 4 | bigint
658+
(5 rows)
659+
660+
SELECT tableoid::regclass, f1, f2, f3, pg_typeof(f3) FROM pitest1_p2;
661+
tableoid | f1 | f2 | f3 | pg_typeof
662+
------------+------------+------------------+-----+-----------
663+
pitest1_p2 | 08-02-2016 | before attaching | 100 | bigint
664+
pitest1_p2 | 08-03-2016 | from pitest1_p2 | 3 | bigint
665+
pitest1_p2 | 08-04-2016 | from pitest1 | 4 | bigint
666+
(3 rows)
667+
640668
-- add identity column
641669
CREATE TABLE pitest2 (f1 date NOT NULL, f2 text) PARTITION BY RANGE (f1);
642670
CREATE TABLE pitest2_p1 PARTITION OF pitest2 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');

‎src/test/regress/sql/identity.sql

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,14 +358,21 @@ CREATE TABLE pitest1_p1 PARTITION OF pitest1 FOR VALUES FROM ('2016-07-01') TO (
358358
INSERT into pitest1(f1, f2) VALUES ('2016-07-2', 'from pitest1');
359359
INSERT into pitest1_p1 (f1, f2) VALUES ('2016-07-3', 'from pitest1_p1');
360360
-- attached partition
361-
CREATE TABLE pitest1_p2 (f1 date NOT NULL, f2 text, f3 bigint);
362-
INSERT INTO pitest1_p2 VALUES ('2016-08-2', 'before attaching', 100);
361+
CREATE TABLE pitest1_p2 (f3 bigint, f2 text, f1 date NOT NULL);
362+
INSERT INTO pitest1_p2 (f1, f2, f3) VALUES ('2016-08-2', 'before attaching', 100);
363363
ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01'); -- requires NOT NULL constraint
364364
ALTER TABLE pitest1_p2 ALTER COLUMN f3 SET NOT NULL;
365365
ALTER TABLE pitest1 ATTACH PARTITION pitest1_p2 FOR VALUES FROM ('2016-08-01') TO ('2016-09-01');
366366
INSERT INTO pitest1_p2 (f1, f2) VALUES ('2016-08-3', 'from pitest1_p2');
367367
INSERT INTO pitest1 (f1, f2) VALUES ('2016-08-4', 'from pitest1');
368+
-- LIKE INCLUDING on partition
369+
CREATE TABLE pitest1_p1_like (LIKE pitest1_p1 INCLUDING IDENTITY);
370+
INSERT into pitest1_p1_like(f1, f2) VALUES ('2016-07-2', 'from pitest1_p1_like');
368371
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1;
372+
SELECT tableoid::regclass, f1, f2, f3 FROM pitest1_p1_like;
373+
ALTER TABLE pitest1 ALTER COLUMN f3 SET DATA TYPE bigint;
374+
SELECT tableoid::regclass, f1, f2, f3, pg_typeof(f3) FROM pitest1;
375+
SELECT tableoid::regclass, f1, f2, f3, pg_typeof(f3) FROM pitest1_p2;
369376

370377
-- add identity column
371378
CREATE TABLE pitest2 (f1 date NOT NULL, f2 text) PARTITION BY RANGE (f1);

0 commit comments

Comments
 (0)