Обсуждение: alter check constraint enforceability

Поиск
Список
Период
Сортировка

alter check constraint enforceability

От
jian he
Дата:
hi.

Currently in pg18, we can add not enforced check constraints.
but we can not do  ALTER TABLE ALTER CONSTRAINT [NOT] ENFORCED
for check constraint.

The attached patch is implementation of changing enforceability of
check constraint.

Вложения

Re: alter check constraint enforceability

От
Robert Treat
Дата:
On Fri, Jul 4, 2025 at 8:00 AM jian he <jian.universality@gmail.com> wrote:
>
> On Mon, Jun 2, 2025 at 9:57 PM jian he <jian.universality@gmail.com> wrote:
> >
> > Currently in pg18, we can add not enforced check constraints.
> > but we can not do  ALTER TABLE ALTER CONSTRAINT [NOT] ENFORCED
> > for check constraint.
> >
> > The attached patch is implementation of changing enforceability of
> > check constraint.
>

Initial look and testing looks good. There are some odd parts to work
through with partitioned tables and recursion (for example, if you
have a parent unenforced, and a child enforced, setting a parent
enforced and then not enforced will recurse to the child, so you end
up in a different state. that could be surprising, but the alternative
is not obviously more sensicle).

Some minor items below:

+ errhint("Only foreign key, check constraint can change enforceability"));

"Only foreign key and check constraints can change enforceability"

--

+ /*
+ * If we are told not to recurse, there had better not be any child
+ * tables, because we can't changing constraint enforceability on
+ * the parent unless we have chaned enforceability for all child
+ * tables.
+ */

* tables, because we can't change constraint enforceability on
* the parent unless we have changed enforceability for all child

--

+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+     cmdcon->is_enforced &&
+     !currcon->conenforced)

i think I have convinced myself that this is correct, but maybe I will
ask you if you had any concerns that this needed to also consider
RELKIND_PARTITIONED_TABLE as well?


Robert Treat
https://xzilla.net



Re: alter check constraint enforceability

От
jian he
Дата:
On Thu, Aug 7, 2025 at 7:35 AM Robert Treat <rob@xzilla.net> wrote:
>
> + if (rel->rd_rel->relkind == RELKIND_RELATION &&
> +     cmdcon->is_enforced &&
> +     !currcon->conenforced)
>
> i think I have convinced myself that this is correct, but maybe I will
> ask you if you had any concerns that this needed to also consider
> RELKIND_PARTITIONED_TABLE as well?
>

ATExecAlterCheckConstrEnforceability itself will be recursive to all
the children.
AlterConstrUpdateConstraintEntry is responsible for changing the catalog state.
except the changing the catalog state, if we change the check
constraint from NOT ENFORCED
to ENFORCED,  we also need to verify it in phase 3.
that's the purpose of
> + if (rel->rd_rel->relkind == RELKIND_RELATION &&
> +     cmdcon->is_enforced &&
> +     !currcon->conenforced)

partitioned tables don't have storage, phase3 table scan to verify
check constraint on partitioned table
don't have effect.

also partitioned table check constraint (name, definition
(pg_constraint.conbin) must match with partition
otherwise partition can be attached to the partitioned table.
so here you don't need to consider RELKIND_PARTITIONED_TABLE.

Вложения

Re: alter check constraint enforceability

От
Kirill Reshke
Дата:
On Mon, 11 Aug 2025 at 14:53, jian he <jian.universality@gmail.com> wrote:
>
> On Thu, Aug 7, 2025 at 7:35 AM Robert Treat <rob@xzilla.net> wrote:
> >
> > + if (rel->rd_rel->relkind == RELKIND_RELATION &&
> > +     cmdcon->is_enforced &&
> > +     !currcon->conenforced)
> >
> > i think I have convinced myself that this is correct, but maybe I will
> > ask you if you had any concerns that this needed to also consider
> > RELKIND_PARTITIONED_TABLE as well?
> >
>
> ATExecAlterCheckConstrEnforceability itself will be recursive to all
> the children.
> AlterConstrUpdateConstraintEntry is responsible for changing the catalog state.
> except the changing the catalog state, if we change the check
> constraint from NOT ENFORCED
> to ENFORCED,  we also need to verify it in phase 3.
> that's the purpose of
> > + if (rel->rd_rel->relkind == RELKIND_RELATION &&
> > +     cmdcon->is_enforced &&
> > +     !currcon->conenforced)
>
> partitioned tables don't have storage, phase3 table scan to verify
> check constraint on partitioned table
> don't have effect.
>
> also partitioned table check constraint (name, definition
> (pg_constraint.conbin) must match with partition
> otherwise partition can be attached to the partitioned table.
> so here you don't need to consider RELKIND_PARTITIONED_TABLE.


Hi!
I looked at v3.

Should we rename `ATExecAlterConstrEnforceability` to
`ATExecAlterFKConstrEnforceability `?


--
Best regards,
Kirill Reshke