Обсуждение: Re: ALTER DOMAIN ADD NOT NULL NOT VALID
On 2025/5/21 18:44, jian he wrote: > hi. > > attached patch is for $subject implementation > > per https://www.postgresql.org/docs/current/sql-alterdomain.html > """ > Although ALTER DOMAIN ADD CONSTRAINT attempts to verify that existing stored > data satisfies the new constraint, this check is not bulletproof, because the > command cannot “see” table rows that are newly inserted or updated and not yet > committed. If there is a hazard that concurrent operations might insert bad > data, the way to proceed is to add the constraint using the NOT VALID option, > commit that command, wait until all transactions started before that commit have > finished, and then issue ALTER DOMAIN VALIDATE CONSTRAINT to search for data > violating the constraint. > """ > > Obviously, the above behavior can also happen to not-null constraints. > add NOT NULL NOT VALID is good for validation invalid data too. > > the not valid information is displayed at column "Nullable" > for example: > > \dD things > List of domains > Schema | Name | Type | Collation | Nullable | Default > | Check > --------+--------+---------+-----------+--------------------+---------+-------------------- > public | things | integer | | not null not valid | > | CHECK (VALUE < 11) It makes sense to support the "NOT NULL NOT VALID" option. The two if statements in the AlterDomainNotNull() should be adjusted. if (typTup->typnotnull == notNull && !notNull) ==> if (!notNull && !typTup->typnotnull) if (typTup->typnotnull == notNull && notNull) ==> if (notNull && typTup->typnotnull) -- Quan Zongliang
On Thu, 14 Aug 2025 at 07:47, jian he <jian.universality@gmail.com> wrote: > > hi. > rebase and minor cosmetic change. Hi! It appears satisfactory to me. I have few observations. One is whether we should now support CREATE DOMAIN ... NOT NULL NOT VALID syntax? This could be a separate patch though. Second observation is just a question: ``` reshke=# create domain dd as int; CREATE DOMAIN reshke=# create table dt(i int, c dd); CREATE TABLE reshke=# insert into dt values(1,null); INSERT 0 1 reshke=# alter domain dd add constraint c not null not valid ; ALTER DOMAIN reshke=# update dt set i = i + 1; UPDATE 1 reshke=# update dt set i = i + 1, c =null; ERROR: domain dd does not allow null values reshke=# table dt; i | c ---+--- 2 | (1 row) ``` Is this behaviour correct? Meaning first update is successful while second is not, yet they would produce the same result. And last is about psql-tab-complete: we now can complete 'alter domain ... add constraint .. not null' with 'not valid'. This also could be a separate patch. -- Best regards, Kirill Reshke
On 2025-Aug-14, Kirill Reshke wrote: > reshke=# create domain dd as int; > CREATE DOMAIN > reshke=# create table dt(i int, c dd); > CREATE TABLE > reshke=# insert into dt values(1,null); > INSERT 0 1 > reshke=# alter domain dd add constraint c not null not valid ; > ALTER DOMAIN > reshke=# update dt set i = i + 1; > UPDATE 1 I think what this example is saying, is that for a not-null constraint on a domain to be really useful, it has to be propagated as a not-null constraint on all table (and matview) columns that are using that domain as datatype. In this case the table column remains nullable after adding the constraint to the domain, which is why no error occurs (and which IMO is bogus). In your other update > reshke=# update dt set i = i + 1, c =null; > ERROR: domain dd does not allow null values the error occurs not when the null value is inserted into the column, but when the value is assigned the domain type. The 2016 SQL standard says in 4.23.4 Domain Constraints: A domain constraint is a constraint that is specified for a domain. It is applied to all columns that are based on that domain, and to all values cast to that domain. which supports the idea that have should do this propagation that I describe. So what I think should happen here, is that if you do ALTER DOMAIN foo ADD CONSTRAINT NOT NULL then we would look up all columns in all tables/matviews that have a column of that datatype, and create another CONSTRAINT_NOTNULL pg_constraint row for that column of that table; then verify all tables/matviews [that don't already have not-null constraints on those columns] and error out if there's a null value in any of them. Contrariwise, and most usefully, if you do ALTER DOMAIN ADD CONSTRAINT NOT NULL NOT VALID then you add all the constraints on each table column and mark them as not-valid; no need for any error here. Then the user can validate each table separately if they want, minimizing time during which tables are locked. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On Thu, Aug 14, 2025 at 5:02 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
>
> I have few observations.
> One is whether we should now support CREATE DOMAIN ... NOT NULL NOT
> VALID syntax? This could be a separate patch though.
>
in gram.y:
CreateDomainStmt:
CREATE DOMAIN_P any_name opt_as Typename ColQualList
{
CreateDomainStmt *n = makeNode(CreateDomainStmt);
n->domainname = $3;
n->typeName = $5;
SplitColQualList($6, &n->constraints, &n->collClause,
yyscanner);
$$ = (Node *) n;
}
;
ColConstraintElem:
NOT NULL_P opt_no_inherit
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->is_no_inherit = $3;
n->is_enforced = true;
n->skip_validation = false;
n->initially_valid = true;
$$ = (Node *) n;
}
| NULL_P
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_NULL;
n->location = @1;
$$ = (Node *) n;
}
CREATE DOMAIN use ColConstraintElem.
that's why we do not support syntax:
``create domain t1 as int not null not valid;``
we also do not support column constraints NOT NULL NOT VALID.
Like
``create table t(a int not null not valid);``
will error out.
so I guess it's fine to not support it?
opt_not_valid: NOT VALID { $$ = true; }
| /* EMPTY */ { $$
= false; }
;
ColConstraintElem:
NOT NULL_P opt_no_inherit opt_not_valid
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->is_no_inherit = $3;
n->is_enforced = true;
// n->skip_validation = false;
// n->initially_valid = true;
n->skip_validation = $4;
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
the above change will produce error
/usr/bin/bison -Wno-deprecated -o src/backend/parser/gram.c -d
../../Desktop/pg_src/src9/postgres/src/backend/parser/gram.y
../../Desktop/pg_src/src9/postgres/src/backend/parser/gram.y: error:
shift/reduce conflicts: 1 found, 0 expected
../../Desktop/pg_src/src9/postgres/src/backend/parser/gram.y: note:
rerun with option '-Wcounterexamples' to generate conflict
counterexamples
so currently I don't know how to support syntax
``create domain t1 as int not null not valid;``
I also found it's hard to psql-tab-complete for
'alter domain ... add constraint .. not null' with 'not valid'.
On Mon, 18 Aug 2025 at 09:09, jian he <jian.universality@gmail.com> wrote:
> CREATE DOMAIN use ColConstraintElem.
> that's why we do not support syntax:
> ``create domain t1 as int not null not valid;``
>
> we also do not support column constraints NOT NULL NOT VALID.
> Like
> ``create table t(a int not null not valid);``
> will error out.
> so I guess it's fine to not support it?
Indeed.
> so currently I don't know how to support syntax
> ``create domain t1 as int not null not valid;``
Ok.
> I also found it's hard to psql-tab-complete for
> 'alter domain ... add constraint .. not null' with 'not valid'.
I am under the impression it is pretty trivial.
I mean, isn't this enough?
```
/* ALTER DOMAIN <sth> ADD CONSTRAINT <sth> NOT NULL */
else if (Matches("ALTER", "DOMAIN", MatchAny, "ADD", "CONSTRAINT",
MatchAny, "NOT", "NULL"))
COMPLETE_WITH("NOT VALID");
```
Anyway, this is purely optional and can be a separate topic.
I propose to focus on Alvaro's feedback for now.
--
Best regards,
Kirill Reshke
hi. please check the attached latest version. I did some minor cosmetic changes. similar to commit 16a0039, we can just use ShareUpdateExclusiveLock to validate existing not-valid not null constraints for the below issue (should the last UPDATE fail or not), I have already created a thread at [1]. -------------- 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; -------------- [1]: https://postgr.es/m/CACJufxE2oFcLmrqDrqJrH5k03fv+v9=+-PBs-mV5WsJ=31XMyw@mail.gmail.com