public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Dean Rasheed <[email protected]>
Cc: Aleksander Alekseev <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Amul Sul <[email protected]>
Cc: vignesh C <[email protected]>
Subject: Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).
Date: Tue, 5 Mar 2024 13:36:11 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAEZATCWZfdZVnsiJdCSZfMNsOSrTtEZ2gjiZ-HQoNc2FMYxGhA@mail.gmail.com>
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)
Attachments:
[text/x-diff] 0001-Don-t-lose-attnotnull-if-a-deferred-PK-supports-it.patch (3.3K, ../[email protected]/2-0001-Don-t-lose-attnotnull-if-a-deferred-PK-supports-it.patch)
download | inline diff:
From cc7032c3f62949d20bc98d4b3c70cb2cb8a0dd5e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <[email protected]>
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 <[email protected]>
Discussion: https://postgr.es/m/CAAJ_b94QonkgsbDXofakHDnORQNgafd1y3Oa5QXfpQNJyXyQ7A@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
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox