v1-0001-refactor-AlterDomainAddConstraint.patch

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