On 2024-Mar-04, Dean Rasheed wrote: > I don't think that this is the right fix. ISTM that the real issue is > that dropping a NOT NULL constraint should not mark the column as > nullable if it is part of a PK, whether or not that PK is deferrable > -- a deferrable PK still marks a column as not nullable.
Yeah. As I said upthread, a good fix seems to require no longer relying on RelationGetIndexAttrBitmap to obtain the columns in the primary key, because that function does not include deferred primary keys. I came up with the attached POC, which seems to fix the reported problem, but of course it needs more polish, a working test case, and verifying whether the new function should be used in more places -- in particular, whether it can be used to revert the changes to RelationGetIndexList that b0e96f311985 did. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "La persona que no quería pecar / estaba obligada a sentarse en duras y empinadas sillas / desprovistas, por cierto de blandos atenuantes" (Patricio Vogel)
>From cc7032c3f62949d20bc98d4b3c70cb2cb8a0dd5e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Tue, 5 Mar 2024 13:32:50 +0100 Subject: [PATCH] Don't lose attnotnull if a deferred PK supports it When dropping a NOT NULL constraint on a column that has a deferred primary key, we would reset attnotnull, but that's bogus. XXX this patch is nowhere near final. Reported-by: Amul Sul <sula...@gmail.com> Discussion: https://postgr.es/m/caaj_b94qonkgsbdxofakhdnorqngafd1y3oa5qxfpqnjyxy...@mail.gmail.com --- src/backend/commands/tablecmds.c | 71 ++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c61f9305c2..84c7871dda 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12704,6 +12704,73 @@ ATExecDropConstraint(Relation rel, const char *constrName, table_close(conrel, RowExclusiveLock); } +/* + * dropconstraint_getpkcols -- subroutine for dropconstraint_internal + * + * This is a workaround to the fact that RelationGetIndexAttrBitmap does + * not consider a DEFERRABLE PRIMARY KEY to be a real primary key. Maybe + * this code should be elsewhere. + */ +static Bitmapset * +dropconstraint_getpkcols(Relation rel) +{ + Relation indrel; + SysScanDesc indscan; + ScanKeyData skey; + HeapTuple htup; + Bitmapset *b = NULL; + + /* + * We try first to obtain a list of PK columns from the cache; if there + * is one, we're done. + */ + b = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_PRIMARY_KEY); + if (b != NULL) + return b; + + /* Prepare to scan pg_index for entries having indrelid = this rel. */ + ScanKeyInit(&skey, + Anum_pg_index_indrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + + indrel = table_open(IndexRelationId, AccessShareLock); + indscan = systable_beginscan(indrel, IndexIndrelidIndexId, true, + NULL, 1, &skey); + + while (HeapTupleIsValid(htup = systable_getnext(indscan))) + { + Form_pg_index index = (Form_pg_index) GETSTRUCT(htup); + + /* + * Ignore any indexes that are currently being dropped and those that + * aren't a primary key. + */ + if (!index->indislive) + continue; + if (!index->indisprimary) + continue; + + /* + * If this primary key was IMMEDIATE, it would have been returned + * by RelationGetIndexAttrBitmap. + */ + Assert(!index->indimmediate); + + for (int i = 0; i < index->indnatts; i++) + { + int attrnum = index->indkey.values[i]; + + b = bms_add_member(b, attrnum - FirstLowInvalidHeapAttributeNumber); + } + } + + systable_endscan(indscan); + table_close(indrel, AccessShareLock); + + return b; +} + /* * Remove a constraint, using its pg_constraint tuple * @@ -12837,9 +12904,7 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha * We want to test columns for their presence in the primary key, but * only if we're not dropping it. */ - pkcols = dropping_pk ? NULL : - RelationGetIndexAttrBitmap(rel, - INDEX_ATTR_BITMAP_PRIMARY_KEY); + pkcols = dropping_pk ? NULL : dropconstraint_getpkcols(rel); ircols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_IDENTITY_KEY); foreach(lc, unconstrained_cols) -- 2.39.2
