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

Thanks Alvaro for the review and fixup patch.

I agree with your changes and merged that into the main patch along with a
couple of other changes.

Please find attached v6 for further review.

--

Thanks & Regards,
Suraj kharage,

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

On Sat, Mar 1, 2025 at 4:15 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:

> On 2025-Feb-21, Suraj Kharage wrote:
>
> > Thanks, Alvaro.
> >
> > I have revised the patch as per your last update.
> > Please find attached v5 for further review.
>
> Hello
>
> I noticed two issues. One is that we are OK to modify a constraint
> that's defined in our parent, which breaks everything. We can only
> allow a top-level constraint to be modified. I added a check in
> ATExecAlterConstraint() for this. Please add a test case for this.
>
> The other one is that when we set a constraint to NO INHERIT on a table
> with children and grandchildren, we must only modify the directly
> inheriting constraints as not having a parent -- we must not recurse to
> also do that in the grandchildren! Otherwise they become disconnected,
> which makes no sense. So we just want to locate the constraint for each
> child, modify by subtracting 1 from coninhcount and set islocal, and
> we're done. The whole ATExecSetNotNullNoInherit() function is based on
> the wrong premise that this requires to recurse. I chose to remove it
> to keep things simple.
>
> Stylistically, in tablecmds.c we always have the 'List **wqueue'
> argument as the first one in functions that take it. So when adding it
> to a function that doesn't have it, don't put it last.
>
> This error message:
> - errmsg("ALTER TABLE \"%s\" ALTER CONSTRAINT \"%s\" SET %s
> only supports not null constraint",
> - RelationGetRelationName(rel), cmdcon->conname,
> - cmdcon->noinherit ? "NO INHERIT" : "INHERIT")));
> seems a bit excessive. Looking at other examples, it doesn't look like
> we need to cite the complete message in so much detail (especially if,
> say, the user specified a schema-qualified table name in the command
> which won't show up in the error message, this would look just weird).
> I simplified it.
>
> Please verify that the tests are still working correctly and resubmit.
>
> --
> Álvaro Herrera PostgreSQL Developer —
> https://www.EnterpriseDB.com/
> "E pur si muove" (Galileo Galilei)
>