Обсуждение: UPDATE with invalid domain constraint
hi. should UPDATE statement need to verify that the domain value is satisfied with invalid domain constraints? Álvaro Herrera already mentioned this in [1], but I just want to confirm it. drop table if exists dt1; drop domain if exists d1; create domain d1 as int; create table dt1(i int, c d1); insert into dt1 values(1,2); alter domain d1 add constraint cc check(value <> 2) not valid; update dt1 set i = i + 1; update dt1 set c = c; update dt1 set i = i + 1, c = c; update dt1 set i = i + 1, c = c::d1; Should the four statements above result in an error? This only happens with UPDATE, since INSERT will check with domain invalid constraints. this ``update dt1 set i = i + 1, c = 2;`` do return error, which is expected. [1]: https://postgr.es/m/202508140957.4daktvyr7xiw@alvherre.pgsql
On Tue, Aug 19, 2025 at 10:08 PM jian he <jian.universality@gmail.com> wrote: > > drop table if exists dt1; > drop domain if exists d1; > create domain d1 as int; > create table dt1(i int, c d1); > insert into dt1 values(1,2); > alter domain d1 add constraint cc check(value <> 2) not valid; > > update dt1 set i = i + 1; > update dt1 set c = c; > update dt1 set i = i + 1, c = c; > update dt1 set i = i + 1, c = c::d1; > > Should the four statements above result in an error? > This only happens with UPDATE, since INSERT will check with domain > invalid constraints. the main idea is that if we find out that a Var Node type is domain with invalid constraint then we convert the Var to CoerceToDomain node. explain (verbose, costs off) update dt1 set i = i + 1; QUERY PLAN ---------------------------------- Update on public.dt1 -> Seq Scan on public.dt1 Output: (i + 1), c, ctid (3 rows) as you can see from the "Output:", column "c" is also here, In rewriteTargetListIU, In rewriteTargetListIU, I use makeTargetEntry to produce a new TargetEntry for column c, set its expr to a CoerceToDomain node, and set resjunk to true.
Вложения
On Aug 20, 2025, at 11:31, jian he <jian.universality@gmail.com> wrote:On Tue, Aug 19, 2025 at 10:08 PM jian he <jian.universality@gmail.com> wrote:
alter domain d1 add constraint cc check(value <> 2) not valid;update dt1 set i = i + 1;
update dt1 set c = c;
update dt1 set i = i + 1, c = c;
update dt1 set i = i + 1, c = c::d1;
I think this is arguably a bug. When “not valid” is given to “alter domain”, it implies the existing invalid data are tolerant, thus if you don’t make any actual change to them, then they should still be tolerant.
I am not sure what other folks’ opinion.
as you can see from the "Output:", column "c" is also here,
In rewriteTargetListIU, In rewriteTargetListIU, I use makeTargetEntry to produce
a new TargetEntry for column c, set its expr to a CoerceToDomain node, and set
resjunk to true.
<v1-0001-UPDATE-with-invalid-domain-constraint.patch>
If this is agreed as a bug, I have a few comment on the patch:
+static Node *
+ConvertVarToCoerceToDomain(Var *var)
+{
+ Oid baseTypeId;
+ int32 baseTypeMod;
+ Node *result;
+
+ baseTypeMod = var->vartypmod;
+ baseTypeId = getBaseTypeAndTypmod(var->vartype, &baseTypeMod);
+ if (baseTypeId != var->vartype &&
+ DomainHasInvalidConstraints(var->vartype))
+ {
I believe " if (baseTypeId != var->vartype” is to make sure the Var is a domain. For that purpose, I think we can use “get_typtype(var->varitype) == TYPTYPE_DOMAIN”.
Also, the function name implies a force converter. I would suggest rename to something like “ConvertVarToCoerceToDomainIfNeed”.
In typcache.c, after line 1101, should we also set typentry->invalidDomainConstr = false.
+++ b/src/backend/utils/cache/typcache.c
@@ -510,6 +510,7 @@ lookup_type_cache(Oid type_id, int flags)
firstDomainTypeEntry = typentry;
}
+ typentry->invalidDomainConstr = false;
This is not needed, as line 487 has MemSet typeentry to all 0.
+ if (IsA(node, Var))
+ {
+ Node *result;
+ result = ConvertVarToCoerceToDomain((Var *) node);
+
+ old_tle->expr = (Expr *) result;
+ }
Should we also set node = result?
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
HighGo Software Co., Ltd.
https://www.highgo.com/