On Wed, Jan 15, 2025 at 12:37 AM Alvaro Herrera <alvhe...@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
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: CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com">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
