Re: Support for NO INHERIT to INHERIT state change with named NOT NULL constraints

From: Suraj Kharage <suraj(dot)kharage(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Support for NO INHERIT to INHERIT state change with named NOT NULL constraints
Date: 2025-02-21 14:10:44
Message-ID: CAF1DzPWDDK6DVbgnt_fbYrgFVrkupmBQTwk+N1eO4YLHEkEFfA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks, Alvaro.

I have revised the patch as per your last update.
Please find attached v5 for further review.

--

Thanks & Regards,
Suraj kharage,

enterprisedb.com <https://www.enterprisedb.com/>

On Wed, Feb 19, 2025 at 9:16 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:

> 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_CKGvz683zFLNeF8gAN5_Q@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)
>

Attachment Content-Type Size
v5-alter_not_null_constraint_to_inherit_no_inherit.patch application/octet-stream 48.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2025-02-21 14:14:13 Re: Doc fix of aggressive vacuum threshold for multixact members storage
Previous Message Sagar Shedge 2025-02-21 13:43:11 Re: Extend postgres_fdw_get_connections to return remote backend pid