Re: UPDATE with invalid domain constraint - Mailing list pgsql-hackers

From Chao Li
Subject Re: UPDATE with invalid domain constraint
Date
Msg-id 7BE9E57C-04A0-41E2-B3D1-074251FA42DE@gmail.com
Whole thread Raw
In response to Re: UPDATE with invalid domain constraint  (jian he <jian.universality@gmail.com>)
List pgsql-hackers


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?


Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




pgsql-hackers by date:

Previous
From: "Zhijie Hou (Fujitsu)"
Date:
Subject: RE: Conflict detection for update_deleted in logical replication
Next
From: Laurenz Albe
Date:
Subject: Re: analyze-in-stages post upgrade questions