Re: Reject ADD CONSTRAINT NOT NULL if name mismatches existing domain not-null constraint

From: ZizhuanLiu X-MAN <44973863(at)qq(dot)com>
To: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, jian(dot)universality <jian(dot)universality(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, 我自己的邮箱 <44973863(at)qq(dot)com>
Subject: Re: Reject ADD CONSTRAINT NOT NULL if name mismatches existing domain not-null constraint
Date: 2026-06-27 02:46:18
Message-ID: tencent_84E11BD212851AB351001EA90D8D96782205@qq.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>On 2026-Mar-02, Chao Li wrote:
>
>> But when I played with the patch, I saw a problem. In the test script, we can see:
>> ```
>> alter domain connotnull add constraint constr1 not null;
>> alter domain connotnull add constraint constr1 not null; — redundant
>> ```
>>
>> If we first create a named constraint “constr1” then create an unnamed one, that’s fine, the unnamed is considered as redundant. However, if I do the reverse order, add a unnamed first, then “constr1”, it failed:
>> ```
>> evantest=# create domain connotnull integer;
>> CREATE DOMAIN
>> evantest=# alter domain connotnull add not null;
>> ALTER DOMAIN
>> evantest=# alter domain connotnull add constraint constr1 not null;
>> ERROR: cannot create not-null constraint "constr1" for domain "connotnull"
>> DETAIL: A not-null constraint named "connotnull_not_null" already exists for domain "connotnull".
>> ```
>>
>> Is that an expected behavior?
>
>Yes. A column or domain can only have one not-null constraint, and the
>default name is not going to match "constr1", so if you request constr1
>first, then that's okay because the second unnamed one matches the
>constraint; but if you request the unnamed first it'll get the default
>name and when you next request "constr1", that won't match the name that
>was defaulted.
>
>--
>álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
>"La espina, desde que nace, ya pincha" (Proverbio africano)

Hi,all

Thanks for the patch, the accompanying tests and your clear explanation.

I have applied the patch and verified it. Its behavior is largely consistent with commit
https://git.postgresql.org/cgit/postgresql.git/commit/?id=96e2af605043974137d84edf5c0a24561956919e.

However, that commit introduced a **specific requirement**: we must reject cases where the explicitly specified
constraint name differs from the name of the existing NOT NULL constraint. Otherwise, subsequent logic would
malfunction, as noted in the commit message:
"Failing to do so could lead to confusion regarding which constraint object actually enforces the rule."

The original behavior of `AlterDomainAddConstraint()` in `typecmd.c` was to return successfully and silently if
the target domain was already NOT NULL. If we now throw an ERROR in this scenario, we will break backward
compatibility for existing application logic, surrounding statement blocks and transactional workflows.

Therefore, I would like to propose the following approach:
emit a NOTICE to indicate the operation is skipped, then return successfully silently while preserving the original
compatible behavior.

I also noticed that `AlterDomainNotNull()` within the same `typecmd.c` has identical logic, so I have extended
this handling to that function as well.

To keep the NOTICE message concise and consistent, I chose not to include the name of the pre-existing constraint.
From the perspective of typical end users such as SQL developers and DBAs, they usually care most about whether
the domain has been successfully set to NOT NULL rather than the internal name of the existing constraint.

On the other hand, experienced developers and DBAs can avoid such issues by relying on anonymous default
constraints via `CREATE DOMAIN ... SET NOT NULL` or `ALTER DOMAIN ... { SET | DROP } NOT NULL`, even though
we cannot enforce this coding practice.

I'd like to share this patch for discussion first. Once we reach an agreement, I will update the test file `domain.sql`
together with its expected output file `domain.out`.

This is my personal proposal, and I welcome further discussion.

Attached please find my local code changes and test results.
```c
AlterDomainAddConstraint()
/* Is the domain already set NOT NULL? */
if (typTup->typnotnull)
{
+ ereport(NOTICE,
+ (errmsg("Domain \"%s\" is already NOT NULL, skipping",
+ NameStr(typTup->typname))));
+
table_close(typrel, RowExclusiveLock);
return address;
}

AlterDomainNotNull()
/* Is the domain already set to the desired constraint? */
if (typTup->typnotnull == notNull)
{
+ ereport(NOTICE,
+ (errmsg("Domain \"%s\" is already %s, skipping",
+ NameStr(typTup->typname), notNull ? "NOT NULL" : "NULL")));
+
table_close(typrel, RowExclusiveLock);
return address;
}
```c

```SQL
--create
xman=# create domain test_domain int not null;
CREATE DOMAIN

--alter add
xman=# alter domain test_domain add not null;
NOTICE: 00000: Domain "test_domain" is already NOT NULL, skipping
LOCATION: AlterDomainAddConstraint, typecmds.c:3040
ALTER DOMAIN
xman=#
xman=# alter domain test_domain add constraint c_not_null not null;
NOTICE: 00000: Domain "test_domain" is already NOT NULL, skipping
LOCATION: AlterDomainAddConstraint, typecmds.c:3040
ALTER DOMAIN
xman=#

--alter set
xman=# alter domain test_domain set not null;
NOTICE: 00000: Domain "test_domain" is already NOT NULL, skipping
LOCATION: AlterDomainNotNull, typecmds.c:2802
ALTER DOMAIN
xman=#
xman=# alter domain test_domain drop not null;
ALTER DOMAIN
xman=#
xman=# alter domain test_domain drop not null;
NOTICE: 00000: Domain "test_domain" is already NULL, skipping
LOCATION: AlterDomainNotNull, typecmds.c:2802
ALTER DOMAIN
xman=#
xman=# alter domain test_domain set not null;
ALTER DOMAIN
xman=#
xman=# alter domain test_domain set not null;
NOTICE: 00000: Domain "test_domain" is already NOT NULL, skipping
LOCATION: AlterDomainNotNull, typecmds.c:2802
ALTER DOMAIN
xman=#
```SQL

regards,
--
ZizhuanLiu (X-MAN) 
44973863(at)qq(dot)com

Attachment Content-Type Size
v2-0001-If-the-domain-is-already-in-the-target-NULL-or-NO.patch.txt application/octet-stream 1.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2026-06-27 05:25:39 statatt_build_stavalues->LOCAL_FCINFO wrong number
Previous Message Tom Lane 2026-06-27 02:44:07 Re: [PATCH] Avoid collation lookup for "char" statistics