On 2025-Feb-10, Suraj Kharage wrote: > Thanks, Alvaro, for the review. > > I have addressed your comments per the above suggestions in the attached v4 > patch.
Okay, thanks. It looks good to me, but I realized a few days ago that this patch affects the same code as the patch from Amul Sul to change enforcedness of constraints[1], and it would be good to structure all these in a sensible manner to avoid creating a mess of routines that work against each other. So I have pushed patch 0001 from Amul, which restructures the way ALTER TABLE .. ALTER CONSTRAINT works. This should make it possible to use the same infrastructure for his NOT ENFORCED constraint change as well as NO INHERIT. The way I see this working for your patch is that you'd remove the new AT_AlterConstraintInherit code I had suggested previously, and instead add a new flag to the ATAlterConstraint struct to carry the information of the state change we want; then the state change would actually be implemented inside ATExecAlterConstraintInternal. I think you'll also need a new member in ATAlterConstraint to carry the column name that's being modified. One detail about that is that the recursion model would have to be different, I think. In the existing code for DEFERRED we simply walk down the hierarchy using the 'conparentid' field to find children. That won't work for INHERIT / NO INHERIT -- for this we need to use normal find_inheritance_children-based recursion. One thing to keep in mind is what happens with ALTER TABLE ONLY parent ALTER CONSTRAINT zzz; ensuring that it operates without recursing for legacy inheritance, and throwing an error for partitioned tables. Thanks! [1] https://postgr.es/m/caaj_b96gevfk5mve5yrvwbuobmfr_ckgvz683zflnef8gan...@mail.gmail.com -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Digital and video cameras have this adjustment and film cameras don't for the same reason dogs and cats lick themselves: because they can." (Ken Rockwell)
