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

On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <
suraj.kharage@enterprisedb.com> wrote:

Alvaro stated that allowing a not null constraint state to be modified
from INHERIT to NO INHERIT is going to be quite problematic because of the
number of weird cases to avoid, so for now that support is not added.

What's the reasoning behind that restriction? What are the weird cases?

--
Robert Haas
EDB: http://www.enterprisedb.com

On Fri, Nov 15, 2024 at 11:15 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <suraj.kharage@enterprisedb.com> wrote:

Alvaro stated that allowing a not null constraint state to be modified from INHERIT to NO INHERIT is going to be quite problematic because of the number of weird cases to avoid, so for now that support is not added.

What's the reasoning behind that restriction? What are the weird cases?

current status:
drop table if exists idxpart,idxpart0,idxpart1 cascade;
create table idxpart (a int not null) partition by list (a);
create table idxpart0 (a int constraint foo not null no inherit);

alter table idxpart attach partition idxpart0 for values in (0,1);
ERROR: constraint "foo" conflicts with non-inherited constraint on
child table "idxpart0"

to make it attach to the partition, we need to drop and recreate the
not-null constraint "foo".
that would be very expensive, since recreate, we need to revalidate
the previous row is not null or not.
related post:
/messages/by-id/202410021219.bvjmxzdspif2@alvherre.pgsql

with
alter table idxpart0 alter constraint foo inherit;

then we can

alter table idxpart attach partition idxpart0 for values in (0,1);

On Thu, Nov 14, 2024 at 1:02 PM Suraj Kharage
<suraj.kharage@enterprisedb.com> wrote:

Hi,

Upstream commit 14e87ffa5c543b5f30ead7413084c25f7735039f added the support for named NOT NULL constraints which are INHERIT by default.
We can declare those as NO INHERIT which means those constraints will not be inherited to child tables and after this state, we don't have the functionality to change the state back to INHERIT.

This patch adds this support where named NOT NULL constraint defined as NO INHERIT can be changed to INHERIT.
For this, introduced the new syntax something like -

ALTER TABLE <tabname> ALTER CONSTRAINT <constrname> INHERIT;

/* ALTER TABLE <name> ALTER CONSTRAINT INHERIT*/
| ALTER CONSTRAINT name INHERIT
{
AlterTableCmd *n = makeNode(AlterTableCmd);
Constraint *c = makeNode(Constraint);

n->subtype = AT_AlterConstraint;
n->def = (Node *) c;
c->contype = CONSTR_NOTNULL;

in gram.y, adding a comment saying this only supports not-null would be great.

comments " * Currently only works for Foreign Key constraints."
above ATExecAlterConstraint need change?

ATExecAlterConstraint
+ if (currcon->contype != CONSTRAINT_NOTNULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("constraint \"%s\" of relation \"%s\" is not a not null constraint",
+ cmdcon->conname, RelationGetRelationName(rel))));
the error message is not helpful?

we should instead saying
ALTER TABLE <tabname> ALTER CONSTRAINT <constrname> INHERIT;
only support not-null constraint.

in ATExecSetNotNull we already have:
if (recurse)
{
List *children;
children = find_inheritance_children(RelationGetRelid(rel),
lockmode);
foreach_oid(childoid, children)
{
Relation childrel = table_open(childoid, NoLock);
CommandCounterIncrement();
ATExecSetNotNull(wqueue, childrel, conName, colName,
recurse, true, lockmode);
table_close(childrel, NoLock);
}
}
so we don't need another CommandCounterIncrement()
in the `foreach_oid(childoid, children)` loop?

maybe we need a CommandCounterIncrement() for
+ /* Update the constraint tuple and mark connoinherit as false. */
+ currcon->connoinherit = false;
+
+ CatalogTupleUpdate(conrel, &contuple->t_self, contuple);
+ ObjectAddressSet(address, ConstraintRelationId, currcon->oid);

+-- error out when provided not null constarint does not exists.
+create table part1(f1 int not null no inherit);
+alter table foo alter constraint part1_id_not_nul inherit;
+ERROR:  constraint "part1_id_not_nul" of relation "foo" does not exist
+drop table part1;
i think you mean:
+alter table part1 alter constraint foo inherit;

--

Thanks & Regards,
Suraj kharage,

enterprisedb.com <https://www.enterprisedb.com/&gt;

On Tue, Nov 19, 2024 at 6:52 PM jian he <jian.universality@gmail.com> wrote:

On Fri, Nov 15, 2024 at 11:15 AM Robert Haas <robertmhaas@gmail.com>
wrote:

On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <

suraj.kharage@enterprisedb.com> wrote:

Alvaro stated that allowing a not null constraint state to be modified

from INHERIT to NO INHERIT is going to be quite problematic because of the
number of weird cases to avoid, so for now that support is not added.

What's the reasoning behind that restriction? What are the weird cases?

current status:
drop table if exists idxpart,idxpart0,idxpart1 cascade;
create table idxpart (a int not null) partition by list (a);
create table idxpart0 (a int constraint foo not null no inherit);

alter table idxpart attach partition idxpart0 for values in (0,1);
ERROR: constraint "foo" conflicts with non-inherited constraint on
child table "idxpart0"

to make it attach to the partition, we need to drop and recreate the
not-null constraint "foo".
that would be very expensive, since recreate, we need to revalidate
the previous row is not null or not.
related post:

/messages/by-id/202410021219.bvjmxzdspif2@alvherre.pgsql

Right.
Another case which needs conclusion is -
When changing from INHERIT to NO INHERIT, we need to walk all children and
decrement coninhcount for the corresponding constraint. If a constraint in
one child reaches zero, should we drop it? not sure. If we do, make sure
to reset the corresponding attnotnull bit too. We could decide not to drop
the constraint, in which case you don’t need to reset attnotnull.

Thanks for the review comments.

On Wed, Nov 20, 2024 at 9:13 AM jian he <jian.universality@gmail.com> wrote:

On Thu, Nov 14, 2024 at 1:02 PM Suraj Kharage
<suraj.kharage@enterprisedb.com> wrote:

Hi,

Upstream commit 14e87ffa5c543b5f30ead7413084c25f7735039f added the

support for named NOT NULL constraints which are INHERIT by default.

We can declare those as NO INHERIT which means those constraints will

not be inherited to child tables and after this state, we don't have the
functionality to change the state back to INHERIT.

This patch adds this support where named NOT NULL constraint defined as

NO INHERIT can be changed to INHERIT.

For this, introduced the new syntax something like -

ALTER TABLE <tabname> ALTER CONSTRAINT <constrname> INHERIT;

/* ALTER TABLE <name> ALTER CONSTRAINT INHERIT*/
| ALTER CONSTRAINT name INHERIT
{
AlterTableCmd *n = makeNode(AlterTableCmd);
Constraint *c = makeNode(Constraint);

n->subtype = AT_AlterConstraint;
n->def = (Node *) c;
c->contype = CONSTR_NOTNULL;

in gram.y, adding a comment saying this only supports not-null would be
great.

Fixed.

comments " * Currently only works for Foreign Key constraints."
above ATExecAlterConstraint need change?

Modified the comment.

ATExecAlterConstraint
+ if (currcon->contype != CONSTRAINT_NOTNULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("constraint \"%s\" of relation \"%s\" is not a not null
constraint",
+ cmdcon->conname, RelationGetRelationName(rel))));
the error message is not helpful?

we should instead saying
ALTER TABLE <tabname> ALTER CONSTRAINT <constrname> INHERIT;
only support not-null constraint.

Modified as per your suggestion.

in ATExecSetNotNull we already have:
if (recurse)
{
List *children;
children = find_inheritance_children(RelationGetRelid(rel),
lockmode);
foreach_oid(childoid, children)
{
Relation childrel = table_open(childoid, NoLock);
CommandCounterIncrement();
ATExecSetNotNull(wqueue, childrel, conName, colName,
recurse, true, lockmode);
table_close(childrel, NoLock);
}
}
so we don't need another CommandCounterIncrement()
in the `foreach_oid(childoid, children)` loop?

I have added that conditional to avoid tuple already updated by self error.
ATExecSetNotNull() is updating tuple and its coninhcount if a constraint
already exists.
Since we have a recursive call to all childrens
from ATExecAlterConstraint(), the recursive call to children doesn't go
through CommandCounterIncrement().

maybe we need a CommandCounterIncrement() for
+ /* Update the constraint tuple and mark connoinherit as false. */
+ currcon->connoinherit = false;
+
+ CatalogTupleUpdate(conrel, &contuple->t_self, contuple);
+ ObjectAddressSet(address, ConstraintRelationId, currcon->oid);

Added.

+-- error out when provided not null constarint does not exists.
+create table part1(f1 int not null no inherit);
+alter table foo alter constraint part1_id_not_nul inherit;
+ERROR:  constraint "part1_id_not_nul" of relation "foo" does not exist
+drop table part1;
i think you mean:
+alter table part1 alter constraint foo inherit;

Fixed.

Please find the attached v2 patch.

On Tue, Nov 19, 2024 at 8:22 AM jian he <jian.universality@gmail.com> wrote:

current status:
drop table if exists idxpart,idxpart0,idxpart1 cascade;
create table idxpart (a int not null) partition by list (a);
create table idxpart0 (a int constraint foo not null no inherit);

alter table idxpart attach partition idxpart0 for values in (0,1);
ERROR: constraint "foo" conflicts with non-inherited constraint on
child table "idxpart0"

to make it attach to the partition, we need to drop and recreate the
not-null constraint "foo".
that would be very expensive, since recreate, we need to revalidate
the previous row is not null or not.

In a simple implementation of ALTER TABLE this would be true, but I
don't see why it should need to be true in ours. It should be possible
to notice that there's an existing NOT NULL constraint and use that as
evidence that the new one can be added without needing to revalidate
the table contents. ALTER TABLE does similar things already. For
instance, TryReuseIndex() can attempt to attach an existing index file
to a new index definition without rebuilding it; TryReuseForeignKey
can attempt to re-add a foreign key constraint without needing to
revalidate it. But even more to the point, ATAddCheckNNConstraint and
MergeWithExistingConstraint know about merging a newly-added
constraint with a preexisting one without needing to revalidate the
table.

--
Robert Haas
EDB: http://www.enterprisedb.com

On 2024-Nov-25, Suraj Kharage wrote:

Another case which needs conclusion is -
When changing from INHERIT to NO INHERIT, we need to walk all children and
decrement coninhcount for the corresponding constraint. If a constraint in
one child reaches zero, should we drop it? not sure. If we do, make sure
to reset the corresponding attnotnull bit too. We could decide not to drop
the constraint, in which case you don’t need to reset attnotnull.

I think it's more useful if we keep such a constraint (but of course
change its conislocal to true, if it isn't that already).

There are arguments for doing both things (drop it or leave it); but if
you drop it, there's no way to put it back without scanning the table
again. If you keep it, it's easy to drop it afterwards.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Postgres is bloatware by design: it was built to house
PhD theses." (Joey Hellerstein, SIGMOD annual conference 2002)

On 2024-Nov-25, Robert Haas wrote:

In a simple implementation of ALTER TABLE this would be true, but I
don't see why it should need to be true in ours. It should be possible
to notice that there's an existing NOT NULL constraint and use that as
evidence that the new one can be added without needing to revalidate
the table contents. ALTER TABLE does similar things already. For
instance, TryReuseIndex() can attempt to attach an existing index file
to a new index definition without rebuilding it; TryReuseForeignKey
can attempt to re-add a foreign key constraint without needing to
revalidate it. But even more to the point, ATAddCheckNNConstraint and
MergeWithExistingConstraint know about merging a newly-added
constraint with a preexisting one without needing to revalidate the
table.

I think you're explaining why we need this patch, which seems a bit
useless in the thread where this patch was posted.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

On Wed, Jan 8, 2025 at 2:43 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

On 2024-Nov-25, Suraj Kharage wrote:

Another case which needs conclusion is -
When changing from INHERIT to NO INHERIT, we need to walk all children

and

decrement coninhcount for the corresponding constraint. If a constraint

in

one child reaches zero, should we drop it? not sure. If we do, make sure
to reset the corresponding attnotnull bit too. We could decide not to

drop

the constraint, in which case you don’t need to reset attnotnull.

I think it's more useful if we keep such a constraint (but of course
change its conislocal to true, if it isn't that already).

There are arguments for doing both things (drop it or leave it); but if
you drop it, there's no way to put it back without scanning the table
again. If you keep it, it's easy to drop it afterwards.

Thanks Alvaro.
Please find attached revised version of patch which added the INHERIT to NO
INHERIT state change for not null constraint.
Keep the constraint (instead of dropping) when we mark NO INHERIT.
As Alvaro mentioned above, we can take others opinion on this behavior.
Also, changed the syntax to ALTER TABLE <tabname> ALTER CONSTRAINT
<constrname> SET INHERIT/NO INHERIT;
just to avoid grammer conflicts but we can decide that as well.

Thanks,
Suraj

On 2025-Jan-13, Suraj Kharage wrote:

Please find attached revised version of patch which added the INHERIT to NO
INHERIT state change for not null constraint.

Thanks!

I find the doc changes a little odd. First, you seem to have added a
[INHERIT/NO INHERIT] flag in the wrong place (line 112); that stuff
already has the NO INHERIT flag next to the constraint types that allow
it, so that change should be removed from the patch. I think the
addition in line 62 are sufficient. Second, adding the explanation for
what this does to the existing varlistentry for ALTER CONSTRAINT looks
out of place. I would add a separate one, something like this perhaps:

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index f9576da435e..10614bcdbd6 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -59,6 +59,7 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     ADD <replaceable class="parameter">table_constraint</replaceable> [ NOT VALID ]
     ADD <replaceable class="parameter">table_constraint_using_index</replaceable>
     ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+    ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> SET [ NO ] INHERIT
     VALIDATE CONSTRAINT <replaceable class="parameter">constraint_name</replaceable>
     DROP CONSTRAINT [ IF EXISTS ]  <replaceable class="parameter">constraint_name</replaceable> [ RESTRICT | CASCADE ]
     DISABLE TRIGGER [ <replaceable class="parameter">trigger_name</replaceable> | ALL | USER ]
@@ -551,7 +552,27 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     <listitem>
      <para>
       This form alters the attributes of a constraint that was previously
-      created. Currently only foreign key constraints may be altered.
+      created. Currently only foreign key constraints may be altered in
+      this fashion, but see below.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry id="sql-altertable-desc-alter-constraint-inherit">
+    <term><literal>ALTER CONSTRAINT ... SET INHERIT</literal></term>
+    <term><literal>ALTER CONSTRAINT ... SET NO INHERIT</literal></term>
+    <listitem>
+     <para>
+      This form modifies a inheritable constraint so that it becomes not
+      inheritable, or vice-versa. Only not-null constraints may be altered
+      in this fashion at present.
+      In addition to changing the inheritability status of the constraint,
+      in the case where a non-inheritable constraint is being marked
+      inheritable, if the table has children, an equivalent constraint
+      is added to them. If marking an inheritable constraint as
+      non-inheritable on a table with children, then the corresponding
+      constraint on children will be marked as no longer inherited,
+      but not removed.
      </para>
     </listitem>
    </varlistentry>

I don't think reusing AT_AlterConstraint for this is a good idea. I
would rather add a new AT_AlterConstraintInherit /
AT_AlterConstraintNoInherit, which takes only a constraint name in
n->name rather than a Constraint in n->def. So gram.y would look like

/*
* ALTER TABLE <name> ALTER CONSTRAINT SET [NO] INHERIT
*/
| ALTER CONSTRAINT name SET INHERIT
{
AlterTableCmd *n = makeNode(AlterTableCmd);

n->subtype = AT_AlterConstraintInherit;
n->name = $3;

$$ = (Node *) n;
}
| ALTER CONSTRAINT name SET NO INHERIT
{
AlterTableCmd *n = makeNode(AlterTableCmd);

n->subtype = AT_AlterConstraintNoInherit;
n->name = $3;

$$ = (Node *) n;
}

This avoids hardcoding in the grammar that we only support this for
not-null constraints -- I'm sure we'll want to implement this for CHECK
constraints later, and at the grammar level there just wouldn't be any
way to implement that the way you have it.

It's a pity that bison doesn't like having unadorned NO INHERIT here.
That would align better with the other use of INHERIT / NO INHERIT we
have in alter table -- requiring a SET there looks ugly. I tried to
change it and the shift/reduce conflict is annoying. I don't have any
bright ideas on fixing that.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Nunca confiaré en un traidor. Ni siquiera si el traidor lo he creado yo"
(Barón Vladimir Harkonnen)

Thanks, Alvaro, for the review.

I have addressed your comments per the above suggestions in the attached v4
patch.

--

Thanks & Regards,
Suraj kharage,

enterprisedb.com <https://www.enterprisedb.com/&gt;

On Wed, Feb 5, 2025 at 12:11 AM Alvaro Herrera <alvherre@alvh.no-ip.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]/messages/by-id/CAAJ_b96gEVfK5MVe5YRVwBuobMFr_CKGvz683zFLNeF8gAN5_Q@mail.gmail.com, 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]: /messages/by-id/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)

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/&gt;

On Wed, Feb 19, 2025 at 9:16 PM Alvaro Herrera <alvherre@alvh.no-ip.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)

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/&gt;

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

On 2025-Mar-03, Suraj Kharage wrote:

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, I have pushed this. I made some changes to the tests, first by
renaming the tables to avoid too generic names, and second to try and
exercise everything about once.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

On 05.03.25 13:56, Alvaro Herrera wrote:

On 2025-Mar-03, Suraj Kharage wrote:

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, I have pushed this. I made some changes to the tests, first by
renaming the tables to avoid too generic names, and second to try and
exercise everything about once.

A patch in the NOT ENFORCED constraints patch series proposes to
refactor some of the code added by this patch series ([0]/messages/by-id/CAAJ_b97aHsJgWhAuRQi1JdWsjzd_ygWEjqQVq_Ddo8dyCnnwkw@mail.gmail.com patch
v18-0001). I noticed that the code paths from this patch series do not
call InvokeObjectPostAlterHook() or CacheInvalidateRelcache() when a
constraint is altered. Was this intentional? If not, I can fix it as
part of that other patch, just wanted to check here.

[0]: /messages/by-id/CAAJ_b97aHsJgWhAuRQi1JdWsjzd_ygWEjqQVq_Ddo8dyCnnwkw@mail.gmail.com
/messages/by-id/CAAJ_b97aHsJgWhAuRQi1JdWsjzd_ygWEjqQVq_Ddo8dyCnnwkw@mail.gmail.com

Hello

On 2025-Mar-25, Peter Eisentraut wrote:

A patch in the NOT ENFORCED constraints patch series proposes to refactor
some of the code added by this patch series ([0] patch v18-0001). I noticed
that the code paths from this patch series do not call
InvokeObjectPostAlterHook() or CacheInvalidateRelcache() when a constraint
is altered. Was this intentional? If not, I can fix it as part of that
other patch, just wanted to check here.

It was not, I'm good with you fixing it, with thanks.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"The ability of users to misuse tools is, of course, legendary" (David Steele)
/messages/by-id/11b38a96-6ded-4668-b772-40f992132797@pgmasters.net

On 25.03.25 12:52, Alvaro Herrera wrote:

Hello

On 2025-Mar-25, Peter Eisentraut wrote:

A patch in the NOT ENFORCED constraints patch series proposes to refactor
some of the code added by this patch series ([0] patch v18-0001). I noticed
that the code paths from this patch series do not call
InvokeObjectPostAlterHook() or CacheInvalidateRelcache() when a constraint
is altered. Was this intentional? If not, I can fix it as part of that
other patch, just wanted to check here.

It was not, I'm good with you fixing it, with thanks.

done