Re: Can't find not null constraint, but \d+ shows that - Mailing list pgsql-hackers

From Tender Wang
Subject Re: Can't find not null constraint, but \d+ shows that
Date
Msg-id CAHewXNnvGv1sbomJRYydVjyMf8SaQzJf6pH_xYTjkHSPDr=UGg@mail.gmail.com
Whole thread Raw
In response to Re: Can't find not null constraint, but \d+ shows that  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers


Alvaro Herrera <alvherre@alvh.no-ip.org> 于2024年4月11日周四 22:48写道:
On 2024-Apr-11, Alvaro Herrera wrote:

> Well, I think you were right that we should try to handle the situation
> of unmarking attnotnull as much as possible, to decrease the chances
> that the problematic situation occurs.  That means, we can use the
> earlier code to handle DROP COLUMN when it causes a PK to be dropped --
> even though we still need to handle the situation of an attnotnull flag
> set with no pg_constraint row.  I mean, we still have to handle DROP
> DOMAIN correctly (and there might be other cases that I haven't thought
> about) ... but surely this is a much less common situation than the one
> you reported.  So I'll merge everything and post an updated patch.

Here's roughly what I'm thinking.  If we drop a constraint, we can still
reset attnotnull in RemoveConstraintById(), but only after checking that
it's not a generated column or a replica identity.  If they are, we
don't error out -- just skip the attnotnull update.

Now, about the code to allow ALTER TABLE DROP NOT NULL in case there's
no pg_constraint row, I think at this point it's mostly dead code,
because it can only happen when you have a replica identity or generated
column ... and the DROP NOT NULL should still prevent you from dropping
the flag anyway.  But the case can still arise, if you change the
replica identity or ALTER TABLE ALTER COLUMN DROP DEFAULT, respectively.

I'm still not ready with this -- still not convinced about the new AT
pass. 

Yeah, at first, I was also hesitant. Two reasons make me convinced.
in ATPostAlterTypeParse()
-----
                               else if (cmd->subtype == AT_SetAttNotNull)
                                {
                                        /*
                                         * The parser will create AT_AttSetNotNull subcommands for
                                         * columns of PRIMARY KEY indexes/constraints, but we need
                                         * not do anything with them here, because the columns'
                                         * NOT NULL marks will already have been propagated into
                                         * the new table definition.
                                         */
                                }
-------
The new table difinition continues to use old column not-null, so here does nothing.
If we reset NOT NULL marks in RemoveConstrainById() when dropping PK indirectly, 
we need to do something here or somewhere else.

Except AT_SetAttNotNull type, other types add a AT pass to tab->subcmds. Because
not-null should be added before re-adding index, there is no right AT pass in current AlterTablePass.
So a new AT pass ahead AT_PASS_OLD_INDEX  is needed.

Another reason is that it can use ALTER TABLE frame to set not-null. 
This way looks simpler and better than hardcode to re-install not-null in some funciton.

--
Tender Wang
OpenPie:  https://en.openpie.com/

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Issue with the PRNG used by Postgres
Next
From: David Steele
Date:
Subject: Re: post-freeze damage control