Skip to content

Commit 0cd7112

Browse files
alvherretenderwg
alvherre
and
tenderwg
committed
Better handle indirect constraint drops
It is possible for certain cases to remove not-null constraints without maintaining the attnotnull in its correct state; for example if you drop a column that's part of the primary key, and the other columns of the PK don't have not-null constraints, then we should reset the attnotnull flags for those other columns; up to this commit, we didn't. Handle those cases better by doing the attnotnull reset in RemoveConstraintById() instead of in dropconstraint_internal(). However, there are some cases where we must not do so. For example if those other columns are in replica identity indexes or are generated identity columns, we must keep attnotnull set, even though it results in the catalog inconsistency that no not-null constraint supports that. Because the attnotnull reset now happens in more places than before, for instance when a column of the primary key changes type, we need an additional trick to reinstate it as necessary. Introduce a new alter-table pass that does this, which needs simply reschedule some AT_SetAttNotNull subcommands that were already being generated and ignored. Because of the exceptions in which attnotnull is not reset noted above, we also include a pg_dump hack to include a not-null constraint when the attnotnull flag is set even if no pg_constraint row exists. This part is undesirable but necessary, because failing to handle the case can result in unrestorable dumps. Reported-by: Tender Wang <tndrwang@gmail.com> Co-authored-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://postgr.es/m/CAHewXN=hMbNa3d43NOR=OCgdgpTt18S-1fmueCoEGesyeK4bqw@mail.gmail.com
1 parent 2e068db commit 0cd7112

File tree

5 files changed

+323
-76
lines changed

5 files changed

+323
-76
lines changed

‎src/backend/catalog/pg_constraint.c

+116-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "access/htup_details.h"
2020
#include "access/sysattr.h"
2121
#include "access/table.h"
22+
#include "access/xact.h"
2223
#include "catalog/catalog.h"
2324
#include "catalog/dependency.h"
2425
#include "catalog/heap.h"
@@ -933,6 +934,8 @@ RemoveConstraintById(Oid conId)
933934
Relation conDesc;
934935
HeapTuple tup;
935936
Form_pg_constraint con;
937+
bool dropping_pk = false;
938+
List *unconstrained_cols = NIL;
936939

937940
conDesc = table_open(ConstraintRelationId, RowExclusiveLock);
938941

@@ -957,7 +960,9 @@ RemoveConstraintById(Oid conId)
957960
/*
958961
* We need to update the relchecks count if it is a check constraint
959962
* being dropped. This update will force backends to rebuild relcache
960-
* entries when we commit.
963+
* entries when we commit. For not-null and primary key constraints,
964+
* obtain the list of columns affected, so that we can reset their
965+
* attnotnull flags below.
961966
*/
962967
if (con->contype == CONSTRAINT_CHECK)
963968
{
@@ -984,6 +989,36 @@ RemoveConstraintById(Oid conId)
984989

985990
table_close(pgrel, RowExclusiveLock);
986991
}
992+
else if (con->contype == CONSTRAINT_NOTNULL)
993+
{
994+
unconstrained_cols = list_make1_int(extractNotNullColumn(tup));
995+
}
996+
else if (con->contype == CONSTRAINT_PRIMARY)
997+
{
998+
Datum adatum;
999+
ArrayType *arr;
1000+
int numkeys;
1001+
bool isNull;
1002+
int16 *attnums;
1003+
1004+
dropping_pk = true;
1005+
1006+
adatum = heap_getattr(tup, Anum_pg_constraint_conkey,
1007+
RelationGetDescr(conDesc), &isNull);
1008+
if (isNull)
1009+
elog(ERROR, "null conkey for constraint %u", con->oid);
1010+
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
1011+
numkeys = ARR_DIMS(arr)[0];
1012+
if (ARR_NDIM(arr) != 1 ||
1013+
numkeys < 0 ||
1014+
ARR_HASNULL(arr) ||
1015+
ARR_ELEMTYPE(arr) != INT2OID)
1016+
elog(ERROR, "conkey is not a 1-D smallint array");
1017+
attnums = (int16 *) ARR_DATA_PTR(arr);
1018+
1019+
for (int i = 0; i < numkeys; i++)
1020+
unconstrained_cols = lappend_int(unconstrained_cols, attnums[i]);
1021+
}
9871022

9881023
/* Keep lock on constraint's rel until end of xact */
9891024
table_close(rel, NoLock);
@@ -1003,6 +1038,86 @@ RemoveConstraintById(Oid conId)
10031038
/* Fry the constraint itself */
10041039
CatalogTupleDelete(conDesc, &tup->t_self);
10051040

1041+
/*
1042+
* If this was a NOT NULL or the primary key, the constrained columns must
1043+
* have had pg_attribute.attnotnull set. See if we need to reset it, and
1044+
* do so.
1045+
*/
1046+
if (unconstrained_cols != NIL)
1047+
{
1048+
Relation tablerel;
1049+
Relation attrel;
1050+
Bitmapset *pkcols;
1051+
ListCell *lc;
1052+
1053+
/* Make the above deletion visible */
1054+
CommandCounterIncrement();
1055+
1056+
tablerel = table_open(con->conrelid, NoLock); /* already have lock */
1057+
attrel = table_open(AttributeRelationId, RowExclusiveLock);
1058+
1059+
/*
1060+
* We want to test columns for their presence in the primary key, but
1061+
* only if we're not dropping it.
1062+
*/
1063+
pkcols = dropping_pk ? NULL :
1064+
RelationGetIndexAttrBitmap(tablerel,
1065+
INDEX_ATTR_BITMAP_PRIMARY_KEY);
1066+
1067+
foreach(lc, unconstrained_cols)
1068+
{
1069+
AttrNumber attnum = lfirst_int(lc);
1070+
HeapTuple atttup;
1071+
HeapTuple contup;
1072+
Bitmapset *ircols;
1073+
Form_pg_attribute attForm;
1074+
1075+
/*
1076+
* Obtain pg_attribute tuple and verify conditions on it. We use
1077+
* a copy we can scribble on.
1078+
*/
1079+
atttup = SearchSysCacheCopyAttNum(con->conrelid, attnum);
1080+
if (!HeapTupleIsValid(atttup))
1081+
elog(ERROR, "cache lookup failed for attribute %d of relation %u",
1082+
attnum, con->conrelid);
1083+
attForm = (Form_pg_attribute) GETSTRUCT(atttup);
1084+
1085+
/*
1086+
* Since the above deletion has been made visible, we can now
1087+
* search for any remaining constraints setting this column as
1088+
* not-nullable; if we find any, no need to reset attnotnull.
1089+
*/
1090+
if (bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber,
1091+
pkcols))
1092+
continue;
1093+
contup = findNotNullConstraintAttnum(con->conrelid, attnum);
1094+
if (contup)
1095+
continue;
1096+
1097+
/*
1098+
* Also no reset if the column is in the replica identity or it's
1099+
* a generated column
1100+
*/
1101+
if (attForm->attidentity != '\0')
1102+
continue;
1103+
ircols = RelationGetIndexAttrBitmap(tablerel,
1104+
INDEX_ATTR_BITMAP_IDENTITY_KEY);
1105+
if (bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber,
1106+
ircols))
1107+
continue;
1108+
1109+
/* Reset attnotnull */
1110+
if (attForm->attnotnull)
1111+
{
1112+
attForm->attnotnull = false;
1113+
CatalogTupleUpdate(attrel, &atttup->t_self, atttup);
1114+
}
1115+
}
1116+
1117+
table_close(attrel, RowExclusiveLock);
1118+
table_close(tablerel, NoLock);
1119+
}
1120+
10061121
/* Clean up */
10071122
ReleaseSysCache(tup);
10081123
table_close(conDesc, RowExclusiveLock);

0 commit comments

Comments
 (0)