>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) [email protected]
v2-0001-If-the-domain-is-already-in-the-target-NULL-or-NO.patch.txt
Description: Binary data
