refactor AlterDomainAddConstraint (alter domain add constraint)
Started by jian heabout 1 year ago11 messages
hi.
attached patch refactor AlterDomainAddConstraint
* change the error message:
alter domain d_int add constraint nn not null no inherit;
from
ERROR: NOT NULL constraints cannot be marked NO INHERIT
to
ERROR: DOMAIN with NOT NULL constraints cannot be marked NO INHERIT
basically processCASbits
from
processCASbits($3, @3, "NOT NULL")
processCASbits($5, @5, "CHECK")
to
processCASbits($3, @3, "DOMAIN with NOT NULL")
processCASbits($5, @5, "DOMAIN with CHECK")
* error out check constraint no inherit with domain. so the following
should fail.
alter domain d_int add constraint cc check(value > 1) no inherit; --should fail
* delete code in AlterDomainAddConstraint, since it will be unreachable.
* alter domain d_int add constraint cc2 check(value > 11) not
deferrable initially immediate not valid;
"not deferrable", "initially immediate" cannot error out at the moment.
maybe we can just document it in create_domain.sgml?
* some failed regress test, similar to thread (Pass ParseState as down
to utility functions)
you may also see the patch draft commit message.
Attachments:
v1-0001-refactor-AlterDomainAddConstraint.patchtext/x-patch; charset=US-ASCII; name=v1-0001-refactor-AlterDomainAddConstraint.patchDownload
From 23186a7ee0b7ebc10ecdab87558d1158676c35d9 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 6 Dec 2024 16:13:51 +0800
Subject: [PATCH v1 1/1] refactor AlterDomainAddConstraint
In gram.y only CHECK and NOT-NULL constraint are allowed for the ALTER DOMAIN ...
ADD CONSTRAINT statement. so we can safely remove the code handles errors for
other constraint type.
as of constraint other property ([ DEFERRABLE | NOT DEFERRABLE ] [
INITIALLY DEFERRED | INITIALLY IMMEDIATE ]) we alerady handled in processCASbits
(gram.y).
however AlterDomainAddConstraint only pass single (Node *newConstraint). that
means we cannot trigger an error for constraints specified as "NOT DEFERRABLE"
or "INITIALLY IMMEDIATE" because "NOT DEFERRABLE" or "INITIALLY IMMEDIATE" is
actualy a seperated Constraint node.
To error out case like:
alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate;
we need to make DomainConstraint in gram.y more like TableConstraint.
that means this patch, "not deferrable" and "initially immediate" words is accepted,
maybe i should change create_domain.sgml Accordingly.
i also made "alter domain d_int add constraint cc check(value > 1) no inherit;"
fail.
the error message is not so good, for example in master, for
`alter domain d_int add constraint nn not null no inherit;`
you get:
ERROR: NOT NULL constraints cannot be marked NO INHERIT
but NOT NULL constraints can be marked NO INHERIT.
for domain constraint part, i slightly changed the third parameter of
processCASbits while calling. so now the error message becomes:
ERROR: DOAMIN with NOT NULL constraints cannot be marked NO INHERIT
---
src/backend/commands/typecmds.c | 33 ----------------
src/backend/parser/gram.y | 6 +--
src/test/regress/expected/domain.out | 58 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 22 +++++++++++
4 files changed, 83 insertions(+), 36 deletions(-)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index da591c0922..aa715402e1 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2942,39 +2942,6 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
/* processed below */
break;
- case CONSTR_UNIQUE:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unique constraints not possible for domains")));
- break;
-
- case CONSTR_PRIMARY:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("primary key constraints not possible for domains")));
- break;
-
- case CONSTR_EXCLUSION:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("exclusion constraints not possible for domains")));
- break;
-
- case CONSTR_FOREIGN:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("foreign key constraints not possible for domains")));
- break;
-
- case CONSTR_ATTR_DEFERRABLE:
- case CONSTR_ATTR_NOT_DEFERRABLE:
- case CONSTR_ATTR_DEFERRED:
- case CONSTR_ATTR_IMMEDIATE:
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("specifying constraint deferrability not supported for domains")));
- break;
-
default:
elog(ERROR, "unrecognized constraint subtype: %d",
(int) constr->contype);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 67eb96396a..ccab075bea 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4321,9 +4321,9 @@ DomainConstraintElem:
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
- processCASbits($5, @5, "CHECK",
+ processCASbits($5, @5, "DOMAIN with CHECK",
NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ NULL, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
@@ -4335,7 +4335,7 @@ DomainConstraintElem:
n->location = @1;
n->keys = list_make1(makeString("value"));
/* no NOT VALID, NO INHERIT support */
- processCASbits($3, @3, "NOT NULL",
+ processCASbits($3, @3, "DOMAIN with NOT NULL",
NULL, NULL, NULL,
NULL, yyscanner);
n->initially_valid = true;
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 42b6559f9c..d6ef6ba098 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -15,6 +15,64 @@ NOTICE: drop cascades to type dependenttypetest
-- this should fail because already gone
drop domain domaindroptest cascade;
ERROR: type "domaindroptest" does not exist
+--alter domain error case.
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked NO INHERIT
+LINE 1: alter domain d_int add constraint nn not null no inherit;
+ ^
+alter domain d_int add constraint nn not null not valid;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked NOT VALID
+LINE 1: alter domain d_int add constraint nn not null not valid;
+ ^
+alter domain d_int add constraint nn not null deferrable;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null deferrable;
+ ^
+alter domain d_int add constraint nn not null initially deferred;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null initially defe...
+ ^
+alter domain d_int add constraint nn not null deferrable initially deferred;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null deferrable ini...
+ ^
+alter domain d_int add constraint nn not null deferrable not deferrable;
+ERROR: conflicting constraint properties
+LINE 1: ...omain d_int add constraint nn not null deferrable not deferr...
+ ^
+alter domain d_int add constraint cc check(value > 1) no inherit;
+ERROR: DOMAIN with CHECK constraints cannot be marked NO INHERIT
+LINE 1: ...r domain d_int add constraint cc check(value > 1) no inherit...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: ...r domain d_int add constraint cc check(value > 1) deferrable...
+ ^
+alter domain d_int add constraint cc check(value > 1) initially deferred;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: ...r domain d_int add constraint cc check(value > 1) initially ...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred not valid;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: ...r domain d_int add constraint cc check(value > 1) deferrable...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable NOT deferrable not valid;
+ERROR: conflicting constraint properties
+LINE 1: ...int add constraint cc check(value > 1) deferrable NOT deferr...
+ ^
+--alter domain valid case.
+alter domain d_int add constraint cc not null not deferrable initially IMMEDIATE; --ok
+alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate; --ok
+alter domain d_int add constraint cc2 check(value > 11) not deferrable initially immediate not valid; --ok
+\dD d_int
+ List of domains
+ Schema | Name | Type | Collation | Nullable | Default | Check
+--------+-------+---------+-----------+----------+---------+------------------------------------------------
+ public | d_int | integer | | not null | | CHECK (VALUE > 1) CHECK (VALUE > 11) NOT VALID
+(1 row)
+
+DROP DOMAIN d_int;
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
-- exercises CoerceToDomain while COPY exercises domain_in.
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index ee07b03174..ae33abea38 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -17,6 +17,28 @@ drop domain domaindroptest cascade;
drop domain domaindroptest cascade;
+--alter domain error case.
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+alter domain d_int add constraint nn not null not valid;
+alter domain d_int add constraint nn not null deferrable;
+alter domain d_int add constraint nn not null initially deferred;
+alter domain d_int add constraint nn not null deferrable initially deferred;
+alter domain d_int add constraint nn not null deferrable not deferrable;
+
+alter domain d_int add constraint cc check(value > 1) no inherit;
+alter domain d_int add constraint cc check(value > 1) deferrable;
+alter domain d_int add constraint cc check(value > 1) initially deferred;
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred not valid;
+alter domain d_int add constraint cc check(value > 1) deferrable NOT deferrable not valid;
+
+--alter domain valid case.
+alter domain d_int add constraint cc not null not deferrable initially IMMEDIATE; --ok
+alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate; --ok
+alter domain d_int add constraint cc2 check(value > 11) not deferrable initially immediate not valid; --ok
+\dD d_int
+DROP DOMAIN d_int;
+
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
--
2.34.1
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On 2024-Dec-06, jian he wrote:
basically processCASbits
from
processCASbits($3, @3, "NOT NULL")
processCASbits($5, @5, "CHECK")
to
processCASbits($3, @3, "DOMAIN with NOT NULL")
processCASbits($5, @5, "DOMAIN with CHECK")
This doesn't actually work from a translation point of view, because the
actual error message is split in two parts. I think it might be better
to pass a non-NULL variable to processCASbits, then in the caller check
whether that flag is true; if so, raise the error in a single ereport().
BTW the way to test this is to apply your patch, then do "make
update-po", then look at the src/backend/po/*.po.new files which contain
the merged strings. In this case, your new "DOMAIN with NOT NULL" string
is not going to appear in the message catalog, because processCASbits()
is not listed in GETTEXT_TRIGGERS in nls.mk.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On Fri, Dec 6, 2024 at 10:48 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2024-Dec-06, jian he wrote:
basically processCASbits
from
processCASbits($3, @3, "NOT NULL")
processCASbits($5, @5, "CHECK")
to
processCASbits($3, @3, "DOMAIN with NOT NULL")
processCASbits($5, @5, "DOMAIN with CHECK")This doesn't actually work from a translation point of view, because the
actual error message is split in two parts. I think it might be better
to pass a non-NULL variable to processCASbits, then in the caller check
whether that flag is true; if so, raise the error in a single ereport().
i let the error fail at AlterDomainAddConstraint.
what do you think?
Attachments:
v2-0001-refactor-AlterDomainAddConstraint.patchtext/x-patch; charset=US-ASCII; name=v2-0001-refactor-AlterDomainAddConstraint.patchDownload
From 106ec073dfae8d5854a948ffc3e89c4fae79e129 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 6 Dec 2024 23:58:34 +0800
Subject: [PATCH v2 1/1] refactor AlterDomainAddConstraint
In gram.y only CHECK and NOT-NULL constraint are allowed for the ALTER DOMAIN ...
ADD CONSTRAINT statement. so we can safely remove the code handles errors for
other constraint type.
as of constraint other property ([ DEFERRABLE | NOT DEFERRABLE ] [
INITIALLY DEFERRED | INITIALLY IMMEDIATE ]) we alerady handled in processCASbits
(gram.y).
however AlterDomainAddConstraint only pass single (Node *newConstraint). that
means we cannot trigger an error for constraints specified as "NOT DEFERRABLE"
or "INITIALLY IMMEDIATE" because "NOT DEFERRABLE" or "INITIALLY IMMEDIATE" is
actualy a seperated Constraint node.
To error out case like:
alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate;
we need to make DomainConstraint in gram.y more like TableConstraint.
that means this patch, "not deferrable" and "initially immediate" words is accepted,
maybe i should change create_domain.sgml Accordingly.
i also made "alter domain d_int add constraint cc check(value > 1) no inherit;"
fail.
the error message is not so good, for example in master, for
`alter domain d_int add constraint nn not null no inherit;`
you get:
ERROR: NOT NULL constraints cannot be marked NO INHERIT
but NOT NULL constraints can be marked NO INHERIT.
for domain constraint part, i slightly changed the third parameter of
processCASbits while calling. so now the error message becomes:
ERROR: DOAMIN with NOT NULL constraints cannot be marked NO INHERIT
discussion: https://postgr.es/m/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com
---
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/typecmds.c | 68 +++++++++++++---------------
src/backend/parser/gram.y | 12 ++---
src/backend/tcop/utility.c | 3 +-
src/include/commands/typecmds.h | 2 +-
src/test/regress/expected/domain.out | 58 ++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 22 +++++++++
7 files changed, 121 insertions(+), 46 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6ccae4cb4a..2dd33c0435 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5390,7 +5390,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
address =
AlterDomainAddConstraint(((AlterDomainStmt *) cmd->def)->typeName,
((AlterDomainStmt *) cmd->def)->def,
- NULL);
+ NULL, context->queryString);
break;
case AT_ReAddComment: /* Re-add existing comment */
address = CommentObject((CommentStmt *) cmd->def);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index da591c0922..96da0fd11d 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2903,7 +2903,7 @@ AlterDomainDropConstraint(List *names, const char *constrName,
*/
ObjectAddress
AlterDomainAddConstraint(List *names, Node *newConstraint,
- ObjectAddress *constrAddr)
+ ObjectAddress *constrAddr, const char *querystring)
{
TypeName *typename;
Oid domainoid;
@@ -2913,10 +2913,13 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
Constraint *constr;
char *ccbin;
ObjectAddress address = InvalidObjectAddress;
+ ParseState *pstate;
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = querystring;
/* Make a TypeName so we can use standard type lookup machinery */
typename = makeTypeNameFromNameList(names);
- domainoid = typenameTypeId(NULL, typename);
+ domainoid = typenameTypeId(pstate, typename);
/* Look up the domain in the type table */
typrel = table_open(TypeRelationId, RowExclusiveLock);
@@ -2938,43 +2941,34 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
switch (constr->contype)
{
case CONSTR_CHECK:
+ if (constr->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("DOMAIN with CHECK constraints cannot be marked NO INHERIT"),
+ parser_errposition(pstate, constr->location));
+ if (constr->deferrable || constr->initdeferred)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("DOMAIN with CHECK constraints cannot be marked DEFERRABLE"),
+ parser_errposition(pstate, constr->location));
+ break;
case CONSTR_NOTNULL:
- /* processed below */
+ if (constr->skip_validation)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("DOMAIN with NOT NULL constraints cannot be marked NOT VALID"),
+ parser_errposition(pstate, constr->location));
+ if (constr->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("DOMAIN with NOT NULL constraints cannot be marked NO INHERIT"),
+ parser_errposition(pstate, constr->location));
+ if (constr->deferrable || constr->initdeferred)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE"),
+ parser_errposition(pstate, constr->location));
break;
-
- case CONSTR_UNIQUE:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unique constraints not possible for domains")));
- break;
-
- case CONSTR_PRIMARY:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("primary key constraints not possible for domains")));
- break;
-
- case CONSTR_EXCLUSION:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("exclusion constraints not possible for domains")));
- break;
-
- case CONSTR_FOREIGN:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("foreign key constraints not possible for domains")));
- break;
-
- case CONSTR_ATTR_DEFERRABLE:
- case CONSTR_ATTR_NOT_DEFERRABLE:
- case CONSTR_ATTR_DEFERRED:
- case CONSTR_ATTR_IMMEDIATE:
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("specifying constraint deferrability not supported for domains")));
- break;
-
default:
elog(ERROR, "unrecognized constraint subtype: %d",
(int) constr->contype);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 67eb96396a..ba90daa265 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4321,8 +4321,8 @@ DomainConstraintElem:
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
- processCASbits($5, @5, "CHECK",
- NULL, NULL, &n->skip_validation,
+ processCASbits($5, @5, "DOMAIN with CHECK",
+ &n->deferrable, &n->initdeferred, &n->skip_validation,
&n->is_no_inherit, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
@@ -4334,10 +4334,10 @@ DomainConstraintElem:
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->keys = list_make1(makeString("value"));
- /* no NOT VALID, NO INHERIT support */
- processCASbits($3, @3, "NOT NULL",
- NULL, NULL, NULL,
- NULL, yyscanner);
+ /* no NOT VALID, NO INHERIT support, error check at AlterDomainAddConstraint */
+ processCASbits($3, @3, "DOMAIN with NOT NULL",
+ &n->deferrable, &n->initdeferred, &n->skip_validation,
+ &n->is_no_inherit, yyscanner);
n->initially_valid = true;
$$ = (Node *) n;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index f28bf37105..ebe24ff8b0 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1367,7 +1367,8 @@ ProcessUtilitySlow(ParseState *pstate,
address =
AlterDomainAddConstraint(stmt->typeName,
stmt->def,
- &secondaryObject);
+ &secondaryObject,
+ queryString);
break;
case 'X': /* DROP CONSTRAINT */
address =
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index e1b02927c4..76b2528b36 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -35,7 +35,7 @@ extern Oid AssignTypeMultirangeArrayOid(void);
extern ObjectAddress AlterDomainDefault(List *names, Node *defaultRaw);
extern ObjectAddress AlterDomainNotNull(List *names, bool notNull);
extern ObjectAddress AlterDomainAddConstraint(List *names, Node *newConstraint,
- ObjectAddress *constrAddr);
+ ObjectAddress *constrAddr, const char* querstring);
extern ObjectAddress AlterDomainValidateConstraint(List *names, const char *constrName);
extern ObjectAddress AlterDomainDropConstraint(List *names, const char *constrName,
DropBehavior behavior, bool missing_ok);
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 42b6559f9c..87ab3f596f 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -15,6 +15,64 @@ NOTICE: drop cascades to type dependenttypetest
-- this should fail because already gone
drop domain domaindroptest cascade;
ERROR: type "domaindroptest" does not exist
+--alter domain error case.
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked NO INHERIT
+LINE 1: alter domain d_int add constraint nn not null no inherit;
+ ^
+alter domain d_int add constraint nn not null not valid;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked NOT VALID
+LINE 1: alter domain d_int add constraint nn not null not valid;
+ ^
+alter domain d_int add constraint nn not null deferrable;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null deferrable;
+ ^
+alter domain d_int add constraint nn not null initially deferred;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null initially defe...
+ ^
+alter domain d_int add constraint nn not null deferrable initially deferred;
+ERROR: DOMAIN with NOT NULL constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint nn not null deferrable ini...
+ ^
+alter domain d_int add constraint nn not null deferrable not deferrable;
+ERROR: conflicting constraint properties
+LINE 1: ...omain d_int add constraint nn not null deferrable not deferr...
+ ^
+alter domain d_int add constraint cc check(value > 1) no inherit;
+ERROR: DOMAIN with CHECK constraints cannot be marked NO INHERIT
+LINE 1: alter domain d_int add constraint cc check(value > 1) no inh...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint cc check(value > 1) deferr...
+ ^
+alter domain d_int add constraint cc check(value > 1) initially deferred;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint cc check(value > 1) initia...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred not valid;
+ERROR: DOMAIN with CHECK constraints cannot be marked DEFERRABLE
+LINE 1: alter domain d_int add constraint cc check(value > 1) deferr...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable NOT deferrable not valid;
+ERROR: conflicting constraint properties
+LINE 1: ...int add constraint cc check(value > 1) deferrable NOT deferr...
+ ^
+--alter domain valid case.
+alter domain d_int add constraint cc not null not deferrable initially IMMEDIATE; --ok
+alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate; --ok
+alter domain d_int add constraint cc2 check(value > 11) not deferrable initially immediate not valid; --ok
+\dD d_int
+ List of domains
+ Schema | Name | Type | Collation | Nullable | Default | Check
+--------+-------+---------+-----------+----------+---------+------------------------------------------------
+ public | d_int | integer | | not null | | CHECK (VALUE > 1) CHECK (VALUE > 11) NOT VALID
+(1 row)
+
+DROP DOMAIN d_int;
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
-- exercises CoerceToDomain while COPY exercises domain_in.
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index ee07b03174..ae33abea38 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -17,6 +17,28 @@ drop domain domaindroptest cascade;
drop domain domaindroptest cascade;
+--alter domain error case.
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+alter domain d_int add constraint nn not null not valid;
+alter domain d_int add constraint nn not null deferrable;
+alter domain d_int add constraint nn not null initially deferred;
+alter domain d_int add constraint nn not null deferrable initially deferred;
+alter domain d_int add constraint nn not null deferrable not deferrable;
+
+alter domain d_int add constraint cc check(value > 1) no inherit;
+alter domain d_int add constraint cc check(value > 1) deferrable;
+alter domain d_int add constraint cc check(value > 1) initially deferred;
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred not valid;
+alter domain d_int add constraint cc check(value > 1) deferrable NOT deferrable not valid;
+
+--alter domain valid case.
+alter domain d_int add constraint cc not null not deferrable initially IMMEDIATE; --ok
+alter domain d_int add constraint cc1 check(value > 1) not deferrable initially immediate; --ok
+alter domain d_int add constraint cc2 check(value > 11) not deferrable initially immediate not valid; --ok
+\dD d_int
+DROP DOMAIN d_int;
+
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
--
2.34.1
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
hi.
ALTER DOMAIN ADD CONSTRAINT syntax more simple than CREATE DOMAIN.
So in gram.y, I changed DomainConstraintElem to the following:
DomainConstraintElem:
CHECK '(' a_expr ')'
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_CHECK;
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
n->initially_valid = true;
$$ = (Node *) n;
}
| CHECK '(' a_expr ')' NOT VALID
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_CHECK;
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
n->initially_valid = false;
n->skip_validation = true;
$$ = (Node *) n;
}
| NOT NULL_P
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->keys = list_make1(makeString("value"));
n->initially_valid = true;
$$ = (Node *) n;
}
Now everything is the same as alter domain synopsis.
It all looks so simple.
Attachments:
v3-0001-refactor-AlterDomainAddConstraint.patchtext/x-patch; charset=US-ASCII; name=v3-0001-refactor-AlterDomainAddConstraint.patchDownload
From e3aa00904edb8481585f7d27b18213f30423696e Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Mon, 9 Dec 2024 16:39:59 +0800
Subject: [PATCH v3 1/1] refactor AlterDomainAddConstraint
In gram.y only CHECK and NOT-NULL constraint are allowed for the ALTER DOMAIN
... ADD CONSTRAINT statement. so we can safely remove the code handles errors
for other constraint type.
further simplify DomainConstraintElem syntax in gram.y. Specifying constraint properties
([ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE | NO INHERIT ])
will be syntax error at parse stage.
in gram.y DomainConstraintElem will be:
CHECK '(' a_expr ')'
| CHECK '(' a_expr ')' NOT VALID
| NOT NULL_P
discussion: https://postgr.es/m/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com
---
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/typecmds.c | 56 +++++---------------------------
src/backend/parser/gram.y | 25 ++++++++------
src/backend/tcop/utility.c | 3 +-
src/include/commands/typecmds.h | 2 +-
5 files changed, 28 insertions(+), 60 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6ccae4cb4a..2dd33c0435 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5390,7 +5390,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
address =
AlterDomainAddConstraint(((AlterDomainStmt *) cmd->def)->typeName,
((AlterDomainStmt *) cmd->def)->def,
- NULL);
+ NULL, context->queryString);
break;
case AT_ReAddComment: /* Re-add existing comment */
address = CommentObject((CommentStmt *) cmd->def);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index da591c0922..71080f1598 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2903,7 +2903,7 @@ AlterDomainDropConstraint(List *names, const char *constrName,
*/
ObjectAddress
AlterDomainAddConstraint(List *names, Node *newConstraint,
- ObjectAddress *constrAddr)
+ ObjectAddress *constrAddr, const char *querystring)
{
TypeName *typename;
Oid domainoid;
@@ -2913,10 +2913,13 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
Constraint *constr;
char *ccbin;
ObjectAddress address = InvalidObjectAddress;
+ ParseState *pstate;
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = querystring;
/* Make a TypeName so we can use standard type lookup machinery */
typename = makeTypeNameFromNameList(names);
- domainoid = typenameTypeId(NULL, typename);
+ domainoid = typenameTypeId(pstate, typename);
/* Look up the domain in the type table */
typrel = table_open(TypeRelationId, RowExclusiveLock);
@@ -2935,51 +2938,10 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
constr = (Constraint *) newConstraint;
- switch (constr->contype)
- {
- case CONSTR_CHECK:
- case CONSTR_NOTNULL:
- /* processed below */
- break;
-
- case CONSTR_UNIQUE:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unique constraints not possible for domains")));
- break;
-
- case CONSTR_PRIMARY:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("primary key constraints not possible for domains")));
- break;
-
- case CONSTR_EXCLUSION:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("exclusion constraints not possible for domains")));
- break;
-
- case CONSTR_FOREIGN:
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("foreign key constraints not possible for domains")));
- break;
-
- case CONSTR_ATTR_DEFERRABLE:
- case CONSTR_ATTR_NOT_DEFERRABLE:
- case CONSTR_ATTR_DEFERRED:
- case CONSTR_ATTR_IMMEDIATE:
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("specifying constraint deferrability not supported for domains")));
- break;
-
- default:
- elog(ERROR, "unrecognized constraint subtype: %d",
- (int) constr->contype);
- break;
- }
+ if (constr->contype != CONSTR_CHECK && constr->contype != CONSTR_NOTNULL)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("domain only support CHECK and NOT-NULL constraints"));
if (constr->contype == CONSTR_CHECK)
{
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 67eb96396a..573f2e8643 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4313,7 +4313,7 @@ DomainConstraint:
;
DomainConstraintElem:
- CHECK '(' a_expr ')' ConstraintAttributeSpec
+ CHECK '(' a_expr ')'
{
Constraint *n = makeNode(Constraint);
@@ -4321,23 +4321,28 @@ DomainConstraintElem:
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
- processCASbits($5, @5, "CHECK",
- NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
- n->initially_valid = !n->skip_validation;
+ n->initially_valid = true;
$$ = (Node *) n;
}
- | NOT NULL_P ConstraintAttributeSpec
+ | CHECK '(' a_expr ')' NOT VALID
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_CHECK;
+ n->location = @1;
+ n->raw_expr = $3;
+ n->cooked_expr = NULL;
+ n->initially_valid = false;
+ n->skip_validation = true;
+ $$ = (Node *) n;
+ }
+ | NOT NULL_P
{
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->keys = list_make1(makeString("value"));
- /* no NOT VALID, NO INHERIT support */
- processCASbits($3, @3, "NOT NULL",
- NULL, NULL, NULL,
- NULL, yyscanner);
n->initially_valid = true;
$$ = (Node *) n;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index f28bf37105..38bb99a750 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1367,7 +1367,8 @@ ProcessUtilitySlow(ParseState *pstate,
address =
AlterDomainAddConstraint(stmt->typeName,
stmt->def,
- &secondaryObject);
+ &secondaryObject,
+ queryString);
break;
case 'X': /* DROP CONSTRAINT */
address =
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index e1b02927c4..76b2528b36 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -35,7 +35,7 @@ extern Oid AssignTypeMultirangeArrayOid(void);
extern ObjectAddress AlterDomainDefault(List *names, Node *defaultRaw);
extern ObjectAddress AlterDomainNotNull(List *names, bool notNull);
extern ObjectAddress AlterDomainAddConstraint(List *names, Node *newConstraint,
- ObjectAddress *constrAddr);
+ ObjectAddress *constrAddr, const char* querstring);
extern ObjectAddress AlterDomainValidateConstraint(List *names, const char *constrName);
extern ObjectAddress AlterDomainDropConstraint(List *names, const char *constrName,
DropBehavior behavior, bool missing_ok);
--
2.34.1
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
Hello,
On 2024-Dec-09, jian he wrote:
ALTER DOMAIN ADD CONSTRAINT syntax more simple than CREATE DOMAIN.
Your proposed patch makes the code simpler, yes, but I think it also
makes the error messages worse. I don't think that's an improvement
from the user point of view.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On Wed, Jan 15, 2025 at 12:37 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Hello,
On 2024-Dec-09, jian he wrote:
ALTER DOMAIN ADD CONSTRAINT syntax more simple than CREATE DOMAIN.
Your proposed patch makes the code simpler, yes, but I think it also
makes the error messages worse. I don't think that's an improvement
from the user point of view.
hi.
thanks for the comments!
we cannot error out AlterDomainAddConstraint for cases like ALTER
DOMAIN ADD CHECK NO INHERIT.
because "NO INHERIT" is actually a separate Constraint Node, and
AlterDomainAddConstraint
can only handle one Constraint node.
i believe I have addressed all the syntax problems related to the
ALTER DOMAIN command.
feel free to try the attached new patch.
examples with master:
create domain d_int as int4;
alter domain d_int add constraint cc check(value > 1) no inherit ;
---ok. success
alter domain d_int add constraint cc check(value > 1) not enforced; --error
ERROR: CHECK constraints cannot be marked NOT ENFORCED
alter domain d_int add constraint cc1 check(value > 1) not deferrable
initially immediate; --ok. success.
--------------------------------------------------------------------------
examples with patch:
alter domain d_int add constraint cc check(value > 1) no inherit;
ERROR: constraint specified as no-inherit is not supported for domains
alter domain d_int add constraint cc check(value > 1) not enforced;
ERROR: specifying constraint enforceability not supported for domains
alter domain d_int add constraint cc1 check(value > 1) not deferrable
initially immediate;
ERROR: specifying constraint deferrability not supported for domains
Attachments:
v4-0001-better-error-message-for-ALTER-DOMAIN-ADD-CONSTRA.patchapplication/x-patch; name=v4-0001-better-error-message-for-ALTER-DOMAIN-ADD-CONSTRA.patchDownload
From c2965313475302a0580000a31640295f5362a998 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 15 Jan 2025 13:18:58 +0800
Subject: [PATCH v4 1/1] better error message for ALTER DOMAIN ADD CONSTRAINT
DomainConstraintElem syntax in gram.y can specify constraint properties such as
([ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE | NO INHERIT ] [ENFORCED | NOT ENFORCED])
According to the ALTER DOMAIN synopsis section, none of these properties are
allowed. In fact, these properties are wrapped up as a separate individual
Constraints node (see DefineDomain). However, AlterDomainAddConstraint can only
cope with a single Constraints node. therefore, error out at
AlterDomainAddConstraint is not possible, so handle error cases in gram.y.
discussion: https://postgr.es/m/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com
---
src/backend/commands/tablecmds.c | 3 +-
src/backend/parser/gram.y | 46 +++++++++++++++++++++++++---
src/test/regress/expected/domain.out | 30 ++++++++++++++++--
src/test/regress/sql/domain.sql | 8 +++++
4 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4fc54bd6eb..34a7ee890d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5405,8 +5405,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
true, true, lockmode);
break;
- case AT_ReAddDomainConstraint: /* Re-add pre-existing domain check
- * constraint */
+ case AT_ReAddDomainConstraint: /* add domain check or not-null constraint */
address =
AlterDomainAddConstraint(((AlterDomainStmt *) cmd->def)->typeName,
((AlterDomainStmt *) cmd->def)->def,
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 6079de70e0..70b612e33d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4343,8 +4343,27 @@ DomainConstraintElem:
n->raw_expr = $3;
n->cooked_expr = NULL;
processCASbits($5, @5, "CHECK",
- NULL, NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ &n->deferrable, &n->initdeferred, &n->is_enforced,
+ &n->skip_validation, &n->is_no_inherit, yyscanner);
+
+ if ($5 & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE | CAS_INITIALLY_IMMEDIATE |
+ CAS_INITIALLY_DEFERRED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@5));
+
+ if ($5 & (CAS_NOT_ENFORCED | CAS_ENFORCED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@5));
+ if (n->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("constraint specified as no-inherit is not supported for domains"),
+ parser_errposition(@5));
+
n->is_enforced = true;
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
@@ -4358,8 +4377,27 @@ DomainConstraintElem:
n->keys = list_make1(makeString("value"));
/* no NOT VALID, NO INHERIT support */
processCASbits($3, @3, "NOT NULL",
- NULL, NULL, NULL,
- NULL, NULL, yyscanner);
+ &n->deferrable, &n->initdeferred, &n->is_enforced,
+ NULL, &n->is_no_inherit, yyscanner);
+
+ if ($3 & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE | CAS_INITIALLY_IMMEDIATE |
+ CAS_INITIALLY_DEFERRED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@3));
+
+ if ($3 & (CAS_NOT_ENFORCED | CAS_ENFORCED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@3));
+ if (n->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("constraint specified as no-inherit is not supported for domains"),
+ parser_errposition(@3));
+
n->initially_valid = true;
$$ = (Node *) n;
}
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index ba6f05eeb7..fe44455162 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -68,6 +68,32 @@ create domain d_fail as int4 constraint cc check (values > 1) deferrable;
ERROR: specifying constraint deferrability not supported for domains
LINE 1: ...n d_fail as int4 constraint cc check (values > 1) deferrable...
^
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+ERROR: constraint specified as no-inherit is not supported for domains
+LINE 1: alter domain d_int add constraint nn not null no inherit;
+ ^
+alter domain d_int add constraint nn not null not enforced;
+ERROR: specifying constraint enforceability not supported for domains
+LINE 1: alter domain d_int add constraint nn not null not enforced;
+ ^
+alter domain d_int add constraint nn not null not deferrable initially immediate;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: alter domain d_int add constraint nn not null not deferrable...
+ ^
+alter domain d_int add constraint cc check(value > 1) no inherit;
+ERROR: constraint specified as no-inherit is not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) no inherit...
+ ^
+alter domain d_int add constraint cc check(value > 1) not enforced;
+ERROR: specifying constraint enforceability not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) not enforc...
+ ^
+alter domain d_int add constraint cc check(value > 1) not deferrable initially immediate;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) not deferr...
+ ^
+drop domain d_int;
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
-- exercises CoerceToDomain while COPY exercises domain_in.
@@ -1363,11 +1389,11 @@ LINE 1: ...S int CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
CREATE DOMAIN constraint_enforced_dom AS int;
-- XXX misleading error messages
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
-ERROR: CHECK constraints cannot be marked ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...om ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
^
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORCED;
-ERROR: CHECK constraints cannot be marked NOT ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
^
DROP DOMAIN constraint_enforced_dom;
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index b752a63ab5..9501ad874d 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -31,6 +31,14 @@ create domain d_fail as int4 constraint cc generated by default as identity;
create domain d_fail as int4 constraint cc check (values > 1) no inherit;
create domain d_fail as int4 constraint cc check (values > 1) deferrable;
+create domain d_int as int4;
+alter domain d_int add constraint nn not null no inherit;
+alter domain d_int add constraint nn not null not enforced;
+alter domain d_int add constraint nn not null not deferrable initially immediate;
+alter domain d_int add constraint cc check(value > 1) no inherit;
+alter domain d_int add constraint cc check(value > 1) not enforced;
+alter domain d_int add constraint cc check(value > 1) not deferrable initially immediate;
+drop domain d_int;
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
--
2.34.1
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
I have committed the part of this patch that removes the switch
statement in AlterDomainAddConstraint(). That one is dead code.
I'll study the other discussion a bit more. I agree the current code
isn't satisfactory.
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
Hello,
On 2025-Jan-15, jian he wrote:
we cannot error out AlterDomainAddConstraint for cases like ALTER
DOMAIN ADD CHECK NO INHERIT.
because "NO INHERIT" is actually a separate Constraint Node, and
AlterDomainAddConstraint
can only handle one Constraint node.
I had forgotten this thread, and I ended up implementing a different
solution for this issue, which I just posted at
/messages/by-id/202503101758.ipn3g64twfye@alvherre.pgsql
I like my patch better than this approach because it allows us to solve
the same problem as it appears in other parts of the grammar, and also
because it avoids the bit fiddling which is harder to maintain later on.
If you'd care to have a look at it, I'd appreciate it.
Thanks
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On 10.03.25 19:37, Alvaro Herrera wrote:
On 2025-Jan-15, jian he wrote:
we cannot error out AlterDomainAddConstraint for cases like ALTER
DOMAIN ADD CHECK NO INHERIT.
because "NO INHERIT" is actually a separate Constraint Node, and
AlterDomainAddConstraint
can only handle one Constraint node.I had forgotten this thread, and I ended up implementing a different
solution for this issue, which I just posted at
/messages/by-id/202503101758.ipn3g64twfye@alvherre.pgsqlI like my patch better than this approach because it allows us to solve
the same problem as it appears in other parts of the grammar, and also
because it avoids the bit fiddling which is harder to maintain later on.
If you'd care to have a look at it, I'd appreciate it.
Where are we on this? Which of the two patches should we pursue?
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On Mon, Nov 24, 2025 at 10:45 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 10.03.25 19:37, Alvaro Herrera wrote:
I had forgotten this thread, and I ended up implementing a different
solution for this issue, which I just posted at
/messages/by-id/202503101758.ipn3g64twfye@alvherre.pgsqlI like my patch better than this approach because it allows us to solve
the same problem as it appears in other parts of the grammar, and also
because it avoids the bit fiddling which is harder to maintain later on.
If you'd care to have a look at it, I'd appreciate it.Where are we on this? Which of the two patches should we pursue?
hi.
if you go to this link
/messages/by-id/202503101758.ipn3g64twfye@alvherre.pgsql
check v2-0001-Improve-processCASbits-API-with-a-seen-struct.patch
you will find that it added a struct CAS_flags to processCASbits.
+typedef struct CAS_flags
+{
+ bool seen_deferrability;
+ bool seen_enforced;
+ bool seen_valid;
+ bool seen_inherit;
+} CAS_flags;In v2-0001, most of the case processCASbits just pass a NULL CAS_flags(seen).
CAS_flags are not used in table constraints at all.
but CAS_flags indeed used for error message handling in ALTER DOMAIN
ADD CONSTRAINT.
(IMHO, it looks like big efforts to solve the same problem, also these bit
fiddling for domain constraint unlikely to change in the future, e.g. we are
unlike to add DEFERRABLE, INITIALLY DEFERRED, NO INHERIT constraints to domain.)
anyway, both patches are attached.
this thread: v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patch
thread: /messages/by-id/202503101758.ipn3g64twfye@alvherre.pgsql
v5-0001-Improve-processCASbits-API-with-a-seen-struct.no-cfbot
v5-0002-handle-constraints-on-domains-too.no-cfbot
Attachments:
v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patchtext/x-patch; charset=US-ASCII; name=v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patchDownload
From 6e9815976016c3aea10f929f7a66251e1d0a57fd Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 26 Nov 2025 12:26:49 +0800
Subject: [PATCH v5 1/1] ALTER DOMAIN ADD CONSTRAINT error message enhance
DomainConstraintElem syntax in gram.y can specify constraint properties such as
([ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE | NO INHERIT ] [ENFORCED | NOT ENFORCED])
ALTER DOMAIN synopsis section indicate none of these properties are allowed.
These properties (DomainConstraintElem) are wrapped up as a separate individual
Constraints node (see DefineDomain).
However, AlterDomainAddConstraint can only cope with a single Constraints node.
therefore, error out at AlterDomainAddConstraint is not possible, so error
message handling stay at gram.y.
discussion: https://postgr.es/m/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com
---
src/backend/parser/gram.y | 53 +++++++++++++++++++++++++---
src/test/regress/expected/domain.out | 42 ++++++++++++++++++++--
src/test/regress/sql/domain.sql | 12 +++++++
3 files changed, 101 insertions(+), 6 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3a0a354a9c..920e611cfb9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4447,8 +4447,28 @@ DomainConstraintElem:
n->raw_expr = $3;
n->cooked_expr = NULL;
processCASbits($5, @5, "CHECK",
- NULL, NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ &n->deferrable, &n->initdeferred, &n->is_enforced,
+ &n->skip_validation, &n->is_no_inherit, yyscanner);
+
+ if ($5 & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE | CAS_INITIALLY_IMMEDIATE |
+ CAS_INITIALLY_DEFERRED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@5));
+
+ if ($5 & (CAS_NOT_ENFORCED | CAS_ENFORCED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@5));
+
+ if (n->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("check constraints for domains cannot be marked %s", "NO INHERIT"),
+ parser_errposition(@5));
+
n->is_enforced = true;
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
@@ -4462,8 +4482,33 @@ DomainConstraintElem:
n->keys = list_make1(makeString("value"));
/* no NOT VALID, NO INHERIT support */
processCASbits($3, @3, "NOT NULL",
- NULL, NULL, NULL,
- NULL, NULL, yyscanner);
+ &n->deferrable, &n->initdeferred, &n->is_enforced,
+ &n->skip_validation, &n->is_no_inherit, yyscanner);
+ if (($3 & CAS_NOT_VALID) != 0)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("not-null constraints on domains cannot be marked %s", "NOT VALID"),
+ parser_errposition(@3));
+
+ if ($3 & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE | CAS_INITIALLY_IMMEDIATE |
+ CAS_INITIALLY_DEFERRED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@3));
+
+ if ($3 & (CAS_NOT_ENFORCED | CAS_ENFORCED))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@3));
+
+ if (n->is_no_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("not-null constraints on domains cannot be marked %s", "NO INHERIT"),
+ parser_errposition(@3));
+
n->initially_valid = true;
$$ = (Node *) n;
}
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 62a48a523a2..357dac1f555 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -68,6 +68,44 @@ create domain d_fail as int4 constraint cc check (values > 1) deferrable;
ERROR: specifying constraint deferrability not supported for domains
LINE 1: ...n d_fail as int4 constraint cc check (values > 1) deferrable...
^
+create domain d_int as int4;
+alter domain d_int add constraint nn not null not valid;
+ERROR: not-null constraints on domains cannot be marked NOT VALID
+LINE 1: alter domain d_int add constraint nn not null not valid;
+ ^
+alter domain d_int add constraint nn not null no inherit;
+ERROR: not-null constraints on domains cannot be marked NO INHERIT
+LINE 1: alter domain d_int add constraint nn not null no inherit;
+ ^
+alter domain d_int add constraint nn not null not enforced;
+ERROR: specifying constraint enforceability not supported for domains
+LINE 1: alter domain d_int add constraint nn not null not enforced;
+ ^
+alter domain d_int add constraint nn not null not deferrable initially immediate;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: alter domain d_int add constraint nn not null not deferrable...
+ ^
+alter domain d_int add constraint cc check(value > 1) no inherit;
+ERROR: check constraints for domains cannot be marked NO INHERIT
+LINE 1: ...r domain d_int add constraint cc check(value > 1) no inherit...
+ ^
+alter domain d_int add constraint cc check(value > 1) not enforced;
+ERROR: specifying constraint enforceability not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) not enforc...
+ ^
+alter domain d_int add constraint cc check(value > 1) enforced;
+ERROR: specifying constraint enforceability not supported for domains
+LINE 1: ...er domain d_int add constraint cc check(value > 1) enforced;
+ ^
+alter domain d_int add constraint cc check(value > 1) not deferrable initially immediate;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) not deferr...
+ ^
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...r domain d_int add constraint cc check(value > 1) deferrable...
+ ^
+drop domain d_int;
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
-- exercises CoerceToDomain while COPY exercises domain_in.
@@ -1369,11 +1407,11 @@ LINE 1: ...S int CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
CREATE DOMAIN constraint_enforced_dom AS int;
-- XXX misleading error messages
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
-ERROR: CHECK constraints cannot be marked ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...om ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
^
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORCED;
-ERROR: CHECK constraints cannot be marked NOT ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
^
DROP DOMAIN constraint_enforced_dom;
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index b8f5a639712..b0c13a75e3a 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -31,6 +31,18 @@ create domain d_fail as int4 constraint cc generated by default as identity;
create domain d_fail as int4 constraint cc check (values > 1) no inherit;
create domain d_fail as int4 constraint cc check (values > 1) deferrable;
+create domain d_int as int4;
+alter domain d_int add constraint nn not null not valid;
+alter domain d_int add constraint nn not null no inherit;
+alter domain d_int add constraint nn not null not enforced;
+alter domain d_int add constraint nn not null not deferrable initially immediate;
+alter domain d_int add constraint cc check(value > 1) no inherit;
+alter domain d_int add constraint cc check(value > 1) not enforced;
+alter domain d_int add constraint cc check(value > 1) enforced;
+alter domain d_int add constraint cc check(value > 1) not deferrable initially immediate;
+alter domain d_int add constraint cc check(value > 1) deferrable initially deferred;
+drop domain d_int;
+
-- Test domain input.
-- Note: the point of checking both INSERT and COPY FROM is that INSERT
--
2.34.1
v5-0001-Improve-processCASbits-API-with-a-seen-struct.no-cfbotapplication/octet-stream; name=v5-0001-Improve-processCASbits-API-with-a-seen-struct.no-cfbotDownload
From b635ac7f4703599b50aa3a4c527356388c19828e Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 26 Nov 2025 11:24:18 +0800
Subject: [PATCH v5 1/2] Improve processCASbits API with a 'seen' struct
This allows ALTER TABLE .. ALTER CONSTRAINT to be more precise about
operations that are supported or not, as well as the reports from CREATE
CONSTRAINT TRIGGER error messages making more sense.
---
src/backend/parser/gram.y | 102 ++++++++++++++++++++++++++++----------
1 file changed, 77 insertions(+), 25 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3a0a354a9c..4e0aa41e2b6 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -147,6 +147,17 @@ typedef struct KeyActions
#define CAS_NOT_ENFORCED 0x40
#define CAS_ENFORCED 0x80
+/*
+ * We represent whether each set of flags is seen on a command with CAS_flags.
+ */
+typedef struct CAS_flags
+{
+ bool seen_deferrability;
+ bool seen_enforced;
+ bool seen_valid;
+ bool seen_inherit;
+} CAS_flags;
+
#define parser_yyerror(msg) scanner_yyerror(msg, yyscanner)
#define parser_errposition(pos) scanner_errposition(pos, yyscanner)
@@ -198,8 +209,9 @@ static void SplitColQualList(List *qualList,
List **constraintList, CollateClause **collClause,
core_yyscan_t yyscanner);
static void processCASbits(int cas_bits, int location, const char *constrType,
- bool *deferrable, bool *initdeferred, bool *is_enforced,
- bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner);
+ bool *deferrable, bool *initdeferred, bool *is_enforced,
+ bool *not_valid, bool *no_inherit, CAS_flags *seen,
+ core_yyscan_t yyscanner);
static PartitionStrategy parsePartitionStrategy(char *strategy, int location,
core_yyscan_t yyscanner);
static void preprocess_pub_all_objtype_list(List *all_objects_list,
@@ -2744,6 +2756,7 @@ alter_table_cmd:
&c->is_enforced,
NULL,
&c->noinherit,
+ NULL,
yyscanner);
$$ = (Node *) n;
}
@@ -4275,7 +4288,7 @@ ConstraintElem:
n->cooked_expr = NULL;
processCASbits($5, @5, "CHECK",
NULL, NULL, &n->is_enforced, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ &n->is_no_inherit, NULL, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
@@ -4288,7 +4301,7 @@ ConstraintElem:
n->keys = list_make1(makeString($3));
processCASbits($4, @4, "NOT NULL",
NULL, NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ &n->is_no_inherit, NULL, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
@@ -4308,7 +4321,7 @@ ConstraintElem:
n->indexspace = $9;
processCASbits($10, @10, "UNIQUE",
&n->deferrable, &n->initdeferred, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
$$ = (Node *) n;
}
| UNIQUE ExistingIndex ConstraintAttributeSpec
@@ -4324,7 +4337,7 @@ ConstraintElem:
n->indexspace = NULL;
processCASbits($3, @3, "UNIQUE",
&n->deferrable, &n->initdeferred, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
$$ = (Node *) n;
}
| PRIMARY KEY '(' columnList opt_without_overlaps ')' opt_c_include opt_definition OptConsTableSpace
@@ -4342,7 +4355,7 @@ ConstraintElem:
n->indexspace = $9;
processCASbits($10, @10, "PRIMARY KEY",
&n->deferrable, &n->initdeferred, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
$$ = (Node *) n;
}
| PRIMARY KEY ExistingIndex ConstraintAttributeSpec
@@ -4358,7 +4371,7 @@ ConstraintElem:
n->indexspace = NULL;
processCASbits($4, @4, "PRIMARY KEY",
&n->deferrable, &n->initdeferred, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
$$ = (Node *) n;
}
| EXCLUDE access_method_clause '(' ExclusionConstraintList ')'
@@ -4378,7 +4391,7 @@ ConstraintElem:
n->where_clause = $9;
processCASbits($10, @10, "EXCLUDE",
&n->deferrable, &n->initdeferred, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
$$ = (Node *) n;
}
| FOREIGN KEY '(' columnList optionalPeriodName ')' REFERENCES qualified_name
@@ -4408,7 +4421,7 @@ ConstraintElem:
processCASbits($12, @12, "FOREIGN KEY",
&n->deferrable, &n->initdeferred,
&n->is_enforced, &n->skip_validation, NULL,
- yyscanner);
+ NULL, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
@@ -4446,9 +4459,9 @@ DomainConstraintElem:
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
- processCASbits($5, @5, "CHECK",
+ processCASbits($5, @5, "CHECK", /* FIXME */
NULL, NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, yyscanner);
+ &n->is_no_inherit, NULL, yyscanner);
n->is_enforced = true;
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
@@ -4461,9 +4474,9 @@ DomainConstraintElem:
n->location = @1;
n->keys = list_make1(makeString("value"));
/* no NOT VALID, NO INHERIT support */
- processCASbits($3, @3, "NOT NULL",
+ processCASbits($3, @3, "NOT NULL", /* FIXME */
NULL, NULL, NULL,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
n->initially_valid = true;
$$ = (Node *) n;
}
@@ -6146,7 +6159,7 @@ CreateTrigStmt:
n->transitionRels = NIL;
processCASbits($11, @11, "TRIGGER",
&n->deferrable, &n->initdeferred, &dummy,
- NULL, NULL, yyscanner);
+ NULL, NULL, NULL, yyscanner);
n->constrrel = $10;
$$ = (Node *) n;
}
@@ -19599,14 +19612,31 @@ SplitColQualList(List *qualList,
}
/*
- * Process result of ConstraintAttributeSpec, and set appropriate bool flags
- * in the output command node. Pass NULL for any flags the particular
- * command doesn't support.
+ * Process result of ConstraintAttributeSpec, and set appropriate bool flags.
+ * Any of those flags can be given as NULL pointers, for options that are
+ * unsupported by the particular production being parsed. If 'seen' is given
+ * as a non NULL pointer, the corresponding boolean there is set for every
+ * option in the command being parsed.
+ *
+ * Unsupported flags for a particular command can be handled in one of two
+ * ways. Productions that require ad-hoc error reporting (those that don't
+ * know which type of constraint is being parsed or simply require a
+ * different phrasing than what this routine provides) can pass a valid
+ * 'seen' pointer; when that is given, each flag in that struct is set when
+ * a particular type of option appears in the command. The production-
+ * specific case can inspect the 'seen' flags and complain appropriately if
+ * one option was seen that the command doesn't support. In this case,
+ * 'constrType' must be given as NULL.
+ *
+ * The other option is to give a NULL 'seen' pointer. In this case, an
+ * unsuported flag will give rise to an error report using the 'constrType',
+ * which must be given as not NULL.
*/
static void
processCASbits(int cas_bits, int location, const char *constrType,
bool *deferrable, bool *initdeferred, bool *is_enforced,
- bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner)
+ bool *not_valid, bool *no_inherit,
+ CAS_flags *seen, core_yyscan_t yyscanner)
{
/* defaults */
if (deferrable)
@@ -19617,70 +19647,90 @@ processCASbits(int cas_bits, int location, const char *constrType,
*not_valid = false;
if (is_enforced)
*is_enforced = true;
+ if (no_inherit)
+ *no_inherit = false;
+
+ /* Assert((constrType == NULL) ^ (seen == NULL)); */
+ if (seen)
+ memset(seen, 0, sizeof(CAS_flags));
if (cas_bits & (CAS_DEFERRABLE | CAS_INITIALLY_DEFERRED))
{
if (deferrable)
*deferrable = true;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked DEFERRABLE",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_deferrability = true;
}
if (cas_bits & CAS_INITIALLY_DEFERRED)
{
if (initdeferred)
*initdeferred = true;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked DEFERRABLE",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_deferrability = true;
}
+ /* not deferrable is the default; just report that we saw it */
+ if (cas_bits & (CAS_NOT_DEFERRABLE) && seen)
+ seen->seen_deferrability = true;
+
if (cas_bits & CAS_NOT_VALID)
{
if (not_valid)
*not_valid = true;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked NOT VALID",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_valid = true;
}
if (cas_bits & CAS_NO_INHERIT)
{
if (no_inherit)
*no_inherit = true;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked NO INHERIT",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_inherit = true;
}
if (cas_bits & CAS_NOT_ENFORCED)
{
if (is_enforced)
*is_enforced = false;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked NOT ENFORCED",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_enforced = true;
/*
* NB: The validated status is irrelevant when the constraint is set to
@@ -19696,13 +19746,15 @@ processCASbits(int cas_bits, int location, const char *constrType,
{
if (is_enforced)
*is_enforced = true;
- else
+ else if (!seen)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
errmsg("%s constraints cannot be marked ENFORCED",
constrType),
parser_errposition(location)));
+ if (seen)
+ seen->seen_enforced = true;
}
}
--
2.34.1
v5-0002-handle-constraints-on-domains-too.no-cfbotapplication/octet-stream; name=v5-0002-handle-constraints-on-domains-too.no-cfbotDownload
From 74117b2b8079ef59f00794ae5169209a1401fd4e Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 26 Nov 2025 11:27:05 +0800
Subject: [PATCH v5 2/2] handle constraints on domains too
source: https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql
---
src/backend/parser/gram.y | 51 +++++++++++++++++++++++++---
src/test/regress/expected/domain.out | 22 ++++++++++--
src/test/regress/sql/domain.sql | 8 ++++-
3 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4e0aa41e2b6..c9a8b375a9f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4454,29 +4454,70 @@ DomainConstraintElem:
CHECK '(' a_expr ')' ConstraintAttributeSpec
{
Constraint *n = makeNode(Constraint);
+ CAS_flags seen;
n->contype = CONSTR_CHECK;
n->location = @1;
n->raw_expr = $3;
n->cooked_expr = NULL;
- processCASbits($5, @5, "CHECK", /* FIXME */
+ processCASbits($5, @5, NULL,
NULL, NULL, NULL, &n->skip_validation,
- &n->is_no_inherit, NULL, yyscanner);
+ &n->is_no_inherit, &seen, yyscanner);
n->is_enforced = true;
+ if (seen.seen_deferrability)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@5));
+ if (seen.seen_enforced)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@5));
+ if (seen.seen_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("CHECK constraints for domains cannot be marked %s",
+ "INHERIT / NO INHERIT"),
+ parser_errposition(@5));
+
n->initially_valid = !n->skip_validation;
$$ = (Node *) n;
}
| NOT NULL_P ConstraintAttributeSpec
{
Constraint *n = makeNode(Constraint);
+ CAS_flags seen;
n->contype = CONSTR_NOTNULL;
n->location = @1;
n->keys = list_make1(makeString("value"));
- /* no NOT VALID, NO INHERIT support */
- processCASbits($3, @3, "NOT NULL", /* FIXME */
+ /* no DEFERRABLE, NOT VALID, NO INHERIT support */
+ processCASbits($3, @3, NULL,
NULL, NULL, NULL,
- NULL, NULL, NULL, yyscanner);
+ NULL, NULL, &seen, yyscanner);
+ if (seen.seen_valid)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("not-null constraints on domains cannot be marked %s",
+ "NOT VALID"),
+ parser_errposition(@3));
+ if (seen.seen_deferrability)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint deferrability not supported for domains"),
+ parser_errposition(@3));
+ if (seen.seen_enforced)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("specifying constraint enforceability not supported for domains"),
+ parser_errposition(@3));
+ if (seen.seen_inherit)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("%s constraints on domains cannot be marked %s",
+ "NOT NULL", "INHERIT / NOT INHERIT"),
+ parser_errposition(@3));
n->initially_valid = true;
$$ = (Node *) n;
}
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 62a48a523a2..5bd182f087a 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1367,15 +1367,31 @@ ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...S int CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
^
CREATE DOMAIN constraint_enforced_dom AS int;
--- XXX misleading error messages
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
-ERROR: CHECK constraints cannot be marked ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...om ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
^
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORCED;
-ERROR: CHECK constraints cannot be marked NOT ENFORCED
+ERROR: specifying constraint enforceability not supported for domains
LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORC...
^
+-- other constraint properties also disallowed
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) DEFERRABLE;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) DEFERRABLE...
+ ^
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT DEFERRABLE;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) NOT DEFERR...
+ ^
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) INITIALLY DEFERRED;
+ERROR: specifying constraint deferrability not supported for domains
+LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) INITIALLY ...
+ ^
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NO INHERIT;
+ERROR: CHECK constraints for domains cannot be marked INHERIT / NO INHERIT
+LINE 1: ...m ADD CONSTRAINT the_constraint CHECK (value > 0) NO INHERIT...
+ ^
DROP DOMAIN constraint_enforced_dom;
--
-- Information schema
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index b8f5a639712..2a75522c60b 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -889,9 +889,15 @@ drop domain mytext cascade;
CREATE DOMAIN constraint_enforced_dom AS int CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
CREATE DOMAIN constraint_not_enforced_dom AS int CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORCED;
CREATE DOMAIN constraint_enforced_dom AS int;
--- XXX misleading error messages
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) ENFORCED;
ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT ENFORCED;
+
+-- other constraint properties also disallowed
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) DEFERRABLE;
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NOT DEFERRABLE;
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) INITIALLY DEFERRED;
+ALTER DOMAIN constraint_enforced_dom ADD CONSTRAINT the_constraint CHECK (value > 0) NO INHERIT;
+
DROP DOMAIN constraint_enforced_dom;
--
--
2.34.1
Re: refactor AlterDomainAddConstraint (alter domain add constraint)
On 2025-Nov-26, jian he wrote:
(IMHO, it looks like big efforts to solve the same problem, also these bit
fiddling for domain constraint unlikely to change in the future, e.g. we are
unlike to add DEFERRABLE, INITIALLY DEFERRED, NO INHERIT constraints to domain.)
True.
I was thinking we would use the this mechanism to solve the issue for
constraint triggers, but in commit 87251e114967 I did that in a
different way. I think this one
this thread: v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patch
is a reasonable implementation, ugly though it is. We could handle the
checks for deferrability before calling processCASbits() though.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Tom: There seems to be something broken here.
Teodor: I'm in sackcloth and ashes... Fixed.
/messages/by-id/482D1632.8010507@sigaev.ru