Skip to content

Commit 270af6f

Browse files
alvherrealvherre
committed
Admit deferrable PKs into rd_pkindex, but flag them as such
... and in particular don't return them as replica identity. The motivation for this change is letting the primary keys be seen by code that derives NOT NULL constraints from them, when creating inheritance children; before this change, if you had a deferrable PK, pg_dump would not recreate the attnotnull marking properly, because the column would not be considered as having anything to back said marking after dropping the throwaway NOT NULL constraint. The reason we don't want these PKs as replica identities is that replication can corrupt data, if the uniqueness constraint is transiently broken. Reported-by: Amul Sul <sulamul@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://postgr.es/m/CAAJ_b94QonkgsbDXofakHDnORQNgafd1y3Oa5QXfpQNJyXyQ7A@mail.gmail.com
1 parent 4c1973f commit 270af6f

File tree

9 files changed

+170
-11
lines changed

9 files changed

+170
-11
lines changed

‎src/backend/replication/logical/relation.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -843,9 +843,9 @@ IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
843843
}
844844

845845
/*
846-
* Get replica identity index or if it is not defined a primary key.
847-
*
848-
* If neither is defined, returns InvalidOid
846+
* Return the OID of the replica identity index if one is defined;
847+
* the OID of the PK if one exists and is not deferrable;
848+
* otherwise, InvalidOid.
849849
*/
850850
Oid
851851
GetRelationIdentityOrPK(Relation rel)

‎src/backend/utils/cache/relcache.c

+20-7
Original file line numberDiff line numberDiff line change
@@ -4761,6 +4761,7 @@ RelationGetIndexList(Relation relation)
47614761
char replident = relation->rd_rel->relreplident;
47624762
Oid pkeyIndex = InvalidOid;
47634763
Oid candidateIndex = InvalidOid;
4764+
bool pkdeferrable = false;
47644765
MemoryContext oldcxt;
47654766

47664767
/* Quick exit if we already computed the list. */
@@ -4802,12 +4803,12 @@ RelationGetIndexList(Relation relation)
48024803
result = lappend_oid(result, index->indexrelid);
48034804

48044805
/*
4805-
* Non-unique, non-immediate or predicate indexes aren't interesting
4806-
* for either oid indexes or replication identity indexes, so don't
4807-
* check them.
4806+
* Non-unique or predicate indexes aren't interesting for either oid
4807+
* indexes or replication identity indexes, so don't check them.
4808+
* Deferred ones are not useful for replication identity either; but
4809+
* we do include them if they are PKs.
48084810
*/
48094811
if (!index->indisunique ||
4810-
!index->indimmediate ||
48114812
!heap_attisnull(htup, Anum_pg_index_indpred, NULL))
48124813
continue;
48134814

@@ -4832,7 +4833,13 @@ RelationGetIndexList(Relation relation)
48324833
if (index->indisprimary &&
48334834
(index->indisvalid ||
48344835
relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
4836+
{
48354837
pkeyIndex = index->indexrelid;
4838+
pkdeferrable = !index->indimmediate;
4839+
}
4840+
4841+
if (!index->indimmediate)
4842+
continue;
48364843

48374844
if (!index->indisvalid)
48384845
continue;
@@ -4854,7 +4861,8 @@ RelationGetIndexList(Relation relation)
48544861
oldlist = relation->rd_indexlist;
48554862
relation->rd_indexlist = list_copy(result);
48564863
relation->rd_pkindex = pkeyIndex;
4857-
if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex))
4864+
relation->rd_ispkdeferrable = pkdeferrable;
4865+
if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex) && !pkdeferrable)
48584866
relation->rd_replidindex = pkeyIndex;
48594867
else if (replident == REPLICA_IDENTITY_INDEX && OidIsValid(candidateIndex))
48604868
relation->rd_replidindex = candidateIndex;
@@ -4957,7 +4965,8 @@ RelationGetStatExtList(Relation relation)
49574965
/*
49584966
* RelationGetPrimaryKeyIndex -- get OID of the relation's primary key index
49594967
*
4960-
* Returns InvalidOid if there is no such index.
4968+
* Returns InvalidOid if there is no such index, or if the primary key is
4969+
* DEFERRABLE.
49614970
*/
49624971
Oid
49634972
RelationGetPrimaryKeyIndex(Relation relation)
@@ -4972,7 +4981,7 @@ RelationGetPrimaryKeyIndex(Relation relation)
49724981
Assert(relation->rd_indexvalid);
49734982
}
49744983

4975-
return relation->rd_pkindex;
4984+
return relation->rd_ispkdeferrable ? InvalidOid : relation->rd_pkindex;
49764985
}
49774986

49784987
/*
@@ -5190,6 +5199,7 @@ RelationGetIndexPredicate(Relation relation)
51905199
* INDEX_ATTR_BITMAP_KEY Columns in non-partial unique indexes not
51915200
* in expressions (i.e., usable for FKs)
51925201
* INDEX_ATTR_BITMAP_PRIMARY_KEY Columns in the table's primary key
5202+
* (beware: even if PK is deferrable!)
51935203
* INDEX_ATTR_BITMAP_IDENTITY_KEY Columns in the table's replica identity
51945204
* index (empty if FULL)
51955205
* INDEX_ATTR_BITMAP_HOT_BLOCKING Columns that block updates from being HOT
@@ -5198,6 +5208,9 @@ RelationGetIndexPredicate(Relation relation)
51985208
* Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that
51995209
* we can include system attributes (e.g., OID) in the bitmap representation.
52005210
*
5211+
* Deferred indexes are considered for the primary key, but not for replica
5212+
* identity.
5213+
*
52015214
* Caller had better hold at least RowExclusiveLock on the target relation
52025215
* to ensure it is safe (deadlock-free) for us to take locks on the relation's
52035216
* indexes. Note that since the introduction of CREATE INDEX CONCURRENTLY,

‎src/include/utils/rel.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ typedef struct RelationData
150150

151151
/* data managed by RelationGetIndexList: */
152152
List *rd_indexlist; /* list of OIDs of indexes on relation */
153-
Oid rd_pkindex; /* OID of primary key, if any */
153+
Oid rd_pkindex; /* OID of (deferrable?) primary key, if any */
154+
bool rd_ispkdeferrable; /* is rd_pkindex a deferrable PK? */
154155
Oid rd_replidindex; /* OID of replica identity index, if any */
155156

156157
/* data managed by RelationGetStatExtList: */

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

+92
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,98 @@ create table cnn2_part1(a int primary key);
10171017
alter table cnn2_parted attach partition cnn2_part1 for values in (1);
10181018
ERROR: column "a" in child table must be marked NOT NULL
10191019
drop table cnn2_parted, cnn2_part1;
1020+
-- columns in regular and LIKE inheritance should be marked not-nullable
1021+
-- for primary keys, even if those are deferred
1022+
CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
1023+
CREATE TABLE notnull_tbl4_lk (LIKE notnull_tbl4);
1024+
CREATE TABLE notnull_tbl4_lk2 (LIKE notnull_tbl4 INCLUDING INDEXES);
1025+
CREATE TABLE notnull_tbl4_lk3 (LIKE notnull_tbl4 INCLUDING INDEXES, CONSTRAINT a_nn NOT NULL a);
1026+
CREATE TABLE notnull_tbl4_cld () INHERITS (notnull_tbl4);
1027+
CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS (notnull_tbl4);
1028+
CREATE TABLE notnull_tbl4_cld3 (PRIMARY KEY (a) DEFERRABLE, CONSTRAINT a_nn NOT NULL a) INHERITS (notnull_tbl4);
1029+
\d+ notnull_tbl4
1030+
Table "public.notnull_tbl4"
1031+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1032+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1033+
a | integer | | not null | | plain | |
1034+
Indexes:
1035+
"notnull_tbl4_pkey" PRIMARY KEY, btree (a) DEFERRABLE INITIALLY DEFERRED
1036+
Child tables: notnull_tbl4_cld,
1037+
notnull_tbl4_cld2,
1038+
notnull_tbl4_cld3
1039+
1040+
\d+ notnull_tbl4_lk
1041+
Table "public.notnull_tbl4_lk"
1042+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1043+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1044+
a | integer | | not null | | plain | |
1045+
Not-null constraints:
1046+
"notnull_tbl4_lk_a_not_null" NOT NULL "a"
1047+
1048+
\d+ notnull_tbl4_lk2
1049+
Table "public.notnull_tbl4_lk2"
1050+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1051+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1052+
a | integer | | not null | | plain | |
1053+
Indexes:
1054+
"notnull_tbl4_lk2_pkey" PRIMARY KEY, btree (a) DEFERRABLE INITIALLY DEFERRED
1055+
1056+
\d+ notnull_tbl4_lk3
1057+
Table "public.notnull_tbl4_lk3"
1058+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1059+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1060+
a | integer | | not null | | plain | |
1061+
Indexes:
1062+
"notnull_tbl4_lk3_pkey" PRIMARY KEY, btree (a) DEFERRABLE INITIALLY DEFERRED
1063+
Not-null constraints:
1064+
"a_nn" NOT NULL "a"
1065+
1066+
\d+ notnull_tbl4_cld
1067+
Table "public.notnull_tbl4_cld"
1068+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1069+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1070+
a | integer | | not null | | plain | |
1071+
Not-null constraints:
1072+
"notnull_tbl4_cld_a_not_null" NOT NULL "a" (inherited)
1073+
Inherits: notnull_tbl4
1074+
1075+
\d+ notnull_tbl4_cld2
1076+
Table "public.notnull_tbl4_cld2"
1077+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1078+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1079+
a | integer | | not null | | plain | |
1080+
Indexes:
1081+
"notnull_tbl4_cld2_pkey" PRIMARY KEY, btree (a) DEFERRABLE
1082+
Not-null constraints:
1083+
"notnull_tbl4_cld2_a_not_null" NOT NULL "a" (inherited)
1084+
Inherits: notnull_tbl4
1085+
1086+
\d+ notnull_tbl4_cld3
1087+
Table "public.notnull_tbl4_cld3"
1088+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1089+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1090+
a | integer | | not null | | plain | |
1091+
Indexes:
1092+
"notnull_tbl4_cld3_pkey" PRIMARY KEY, btree (a) DEFERRABLE
1093+
Not-null constraints:
1094+
"a_nn" NOT NULL "a" (local, inherited)
1095+
Inherits: notnull_tbl4
1096+
1097+
-- leave these tables around for pg_upgrade testing
1098+
-- also, if a NOT NULL is dropped underneath a deferrable PK, the column
1099+
-- should still be nullable afterwards. This mimics what pg_dump does.
1100+
CREATE TABLE notnull_tbl5 (a INTEGER CONSTRAINT a_nn NOT NULL);
1101+
ALTER TABLE notnull_tbl5 ADD PRIMARY KEY (a) DEFERRABLE;
1102+
ALTER TABLE notnull_tbl5 DROP CONSTRAINT a_nn;
1103+
\d+ notnull_tbl5
1104+
Table "public.notnull_tbl5"
1105+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
1106+
--------+---------+-----------+----------+---------+---------+--------------+-------------
1107+
a | integer | | not null | | plain | |
1108+
Indexes:
1109+
"notnull_tbl5_pkey" PRIMARY KEY, btree (a) DEFERRABLE
1110+
1111+
DROP TABLE notnull_tbl5;
10201112
-- Comments
10211113
-- Setup a low-level role to enforce non-superuser checks.
10221114
CREATE ROLE regress_constraint_comments;

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

+10
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,16 @@ ALTER PUBLICATION testpub_table_ins ADD TABLE testpub_tbl5 (a); -- ok
732732
Tables:
733733
"public.testpub_tbl5" (a)
734734

735+
-- error: cannot work with deferrable primary keys
736+
CREATE TABLE testpub_tbl5d (a int PRIMARY KEY DEFERRABLE);
737+
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5d;
738+
UPDATE testpub_tbl5d SET a = 1;
739+
ERROR: cannot update table "testpub_tbl5d" because it does not have a replica identity and publishes updates
740+
HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.
741+
/* but works fine with FULL replica identity */
742+
ALTER TABLE testpub_tbl5d REPLICA IDENTITY FULL;
743+
UPDATE testpub_tbl5d SET a = 1;
744+
DROP TABLE testpub_tbl5d;
735745
-- tests with REPLICA IDENTITY FULL
736746
CREATE TABLE testpub_tbl6 (a int, b text, c text);
737747
ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;

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

+5
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ CREATE TABLE test_replica_identity (
77
CONSTRAINT test_replica_identity_unique_nondefer UNIQUE (keya, keyb)
88
) ;
99
CREATE TABLE test_replica_identity_othertable (id serial primary key);
10+
CREATE TABLE test_replica_identity_t3 (id serial constraint pk primary key deferrable);
1011
CREATE INDEX test_replica_identity_keyab ON test_replica_identity (keya, keyb);
1112
CREATE UNIQUE INDEX test_replica_identity_keyab_key ON test_replica_identity (keya, keyb);
1213
CREATE UNIQUE INDEX test_replica_identity_nonkey ON test_replica_identity (keya, nonkey);
@@ -57,6 +58,9 @@ ERROR: "test_replica_identity_othertable_pkey" is not an index for table "test_
5758
-- fail, deferrable
5859
ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_identity_unique_defer;
5960
ERROR: cannot use non-immediate index "test_replica_identity_unique_defer" as replica identity
61+
-- fail, deferrable
62+
ALTER TABLE test_replica_identity_t3 REPLICA IDENTITY USING INDEX pk;
63+
ERROR: cannot use non-immediate index "pk" as replica identity
6064
SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
6165
relreplident
6266
--------------
@@ -292,3 +296,4 @@ DROP TABLE test_replica_identity3;
292296
DROP TABLE test_replica_identity4;
293297
DROP TABLE test_replica_identity5;
294298
DROP TABLE test_replica_identity_othertable;
299+
DROP TABLE test_replica_identity_t3;

‎src/test/regress/sql/constraints.sql

+25
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,31 @@ create table cnn2_part1(a int primary key);
668668
alter table cnn2_parted attach partition cnn2_part1 for values in (1);
669669
drop table cnn2_parted, cnn2_part1;
670670

671+
-- columns in regular and LIKE inheritance should be marked not-nullable
672+
-- for primary keys, even if those are deferred
673+
CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED);
674+
CREATE TABLE notnull_tbl4_lk (LIKE notnull_tbl4);
675+
CREATE TABLE notnull_tbl4_lk2 (LIKE notnull_tbl4 INCLUDING INDEXES);
676+
CREATE TABLE notnull_tbl4_lk3 (LIKE notnull_tbl4 INCLUDING INDEXES, CONSTRAINT a_nn NOT NULL a);
677+
CREATE TABLE notnull_tbl4_cld () INHERITS (notnull_tbl4);
678+
CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS (notnull_tbl4);
679+
CREATE TABLE notnull_tbl4_cld3 (PRIMARY KEY (a) DEFERRABLE, CONSTRAINT a_nn NOT NULL a) INHERITS (notnull_tbl4);
680+
\d+ notnull_tbl4
681+
\d+ notnull_tbl4_lk
682+
\d+ notnull_tbl4_lk2
683+
\d+ notnull_tbl4_lk3
684+
\d+ notnull_tbl4_cld
685+
\d+ notnull_tbl4_cld2
686+
\d+ notnull_tbl4_cld3
687+
-- leave these tables around for pg_upgrade testing
688+
689+
-- also, if a NOT NULL is dropped underneath a deferrable PK, the column
690+
-- should still be nullable afterwards. This mimics what pg_dump does.
691+
CREATE TABLE notnull_tbl5 (a INTEGER CONSTRAINT a_nn NOT NULL);
692+
ALTER TABLE notnull_tbl5 ADD PRIMARY KEY (a) DEFERRABLE;
693+
ALTER TABLE notnull_tbl5 DROP CONSTRAINT a_nn;
694+
\d+ notnull_tbl5
695+
DROP TABLE notnull_tbl5;
671696

672697
-- Comments
673698
-- Setup a low-level role to enforce non-superuser checks.

‎src/test/regress/sql/publication.sql

+9
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,15 @@ RESET client_min_messages;
444444
ALTER PUBLICATION testpub_table_ins ADD TABLE testpub_tbl5 (a); -- ok
445445
\dRp+ testpub_table_ins
446446

447+
-- error: cannot work with deferrable primary keys
448+
CREATE TABLE testpub_tbl5d (a int PRIMARY KEY DEFERRABLE);
449+
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5d;
450+
UPDATE testpub_tbl5d SET a = 1;
451+
/* but works fine with FULL replica identity */
452+
ALTER TABLE testpub_tbl5d REPLICA IDENTITY FULL;
453+
UPDATE testpub_tbl5d SET a = 1;
454+
DROP TABLE testpub_tbl5d;
455+
447456
-- tests with REPLICA IDENTITY FULL
448457
CREATE TABLE testpub_tbl6 (a int, b text, c text);
449458
ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;

‎src/test/regress/sql/replica_identity.sql

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CREATE TABLE test_replica_identity (
88
) ;
99

1010
CREATE TABLE test_replica_identity_othertable (id serial primary key);
11+
CREATE TABLE test_replica_identity_t3 (id serial constraint pk primary key deferrable);
1112

1213
CREATE INDEX test_replica_identity_keyab ON test_replica_identity (keya, keyb);
1314
CREATE UNIQUE INDEX test_replica_identity_keyab_key ON test_replica_identity (keya, keyb);
@@ -40,6 +41,8 @@ ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_iden
4041
ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_identity_othertable_pkey;
4142
-- fail, deferrable
4243
ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_identity_unique_defer;
44+
-- fail, deferrable
45+
ALTER TABLE test_replica_identity_t3 REPLICA IDENTITY USING INDEX pk;
4346

4447
SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
4548

@@ -137,3 +140,4 @@ DROP TABLE test_replica_identity3;
137140
DROP TABLE test_replica_identity4;
138141
DROP TABLE test_replica_identity5;
139142
DROP TABLE test_replica_identity_othertable;
143+
DROP TABLE test_replica_identity_t3;

0 commit comments

Comments
 (0)