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 |
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 |