On Wed, Apr 10, 2024 at 7:01 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
>
> On 2024-Apr-10, jian he wrote:
>
> > another related bug, in master.
> >
> > drop table if exists notnull_tbl1;
> > CREATE TABLE notnull_tbl1 (c0 int not null, c1 int);
> > ALTER TABLE notnull_tbl1 ADD CONSTRAINT Q PRIMARY KEY(c0, c1);
> > \d+ notnull_tbl1
> > ALTER TABLE notnull_tbl1 ALTER c0 DROP NOT NULL;
> > ALTER TABLE notnull_tbl1 ALTER c1 DROP NOT NULL;
> >
> > "ALTER TABLE notnull_tbl1 ALTER c0 DROP NOT NULL;"
> > should fail?
>
> No, this should not fail, and it is working correctly in master. You
> can drop the not-null constraint, but the column will still be
> non-nullable, because the primary key still exists. If you drop the
> primary key later, then the column becomes nullable. This is by design.
>
now I got it. the second time, it will fail.
it should be the expected behavior.
per commit:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=14dd0f27d7cd56ffae9ecdbe324965073d01a9ff
In the function dropconstraint_internal, I changed "foreach" to
"foreach_int" in some places,
and other minor cosmetic changes within the function
dropconstraint_internal only.
Since I saw your changes in dropconstraint_internal, I posted here.
I will review your latest patch later.
From dd41bf8d1f2dea5725909150609f8e565e11efcf Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 10 Apr 2024 20:49:04 +0800
Subject: [PATCH v1 1/1] minor coesmetuic refactor in dropconstraint_internal
mainly change `foreach` to `foreach_int` while iterating through list unconstrained_cols.
---
src/backend/commands/tablecmds.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a02c5b0..8dc623e8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12968,12 +12968,11 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
* have had pg_attribute.attnotnull set. See if we need to reset it, and
* do so.
*/
- if (unconstrained_cols)
+ if (unconstrained_cols != NIL)
{
Relation attrel;
Bitmapset *pkcols;
Bitmapset *ircols;
- ListCell *lc;
/* Make the above deletion visible */
CommandCounterIncrement();
@@ -12989,9 +12988,8 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
INDEX_ATTR_BITMAP_PRIMARY_KEY);
ircols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_IDENTITY_KEY);
- foreach(lc, unconstrained_cols)
+ foreach_int(attnum, unconstrained_cols)
{
- AttrNumber attnum = lfirst_int(lc);
HeapTuple atttup;
HeapTuple contup;
Form_pg_attribute attForm;
@@ -13039,11 +13037,11 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
* It's not valid to drop the not-null constraint for a column in
* the replica identity index, either. (FULL is not affected.)
*/
- if (bms_is_member(lfirst_int(lc) - FirstLowInvalidHeapAttributeNumber, ircols))
+ if (bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber, ircols))
ereport(ERROR,
errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("column \"%s\" is in index used as replica identity",
- get_attname(RelationGetRelid(rel), lfirst_int(lc), false)));
+ get_attname(RelationGetRelid(rel), attnum, false)));
/* Reset attnotnull */
if (attForm->attnotnull)
@@ -13233,10 +13231,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
* Find out the list of column names to process. Fortunately, we
* already have the list of column numbers.
*/
- foreach(lc, unconstrained_cols)
+ foreach_int(attnum, unconstrained_cols)
{
colnames = lappend(colnames, get_attname(RelationGetRelid(rel),
- lfirst_int(lc), false));
+ attnum, false));
}
foreach(child, children)
--
2.34.1
