From 4e4de402dda81798bb0286a09d39c6ed2f087228 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 8 Apr 2024 11:39:48 +0200 Subject: [PATCH v2] Fix ALTER DOMAIN NOT NULL syntax In CREATE DOMAIN, a NOT NULL constraint looks like CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL (Before e5da0fe3c22, the constraint name was accepted but ignored.) But in ALTER DOMAIN, a NOT NULL constraint looks like ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE where VALUE is where for a table constraint the column name would be. (This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax resulted in an internal error.) But for domains, this latter syntax is confusing and needlessly inconsistent between CREATE and ALTER. So this changes it to just ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL (None of these syntaxes are per SQL standard; we are just living with the bits of inconsistency that have built up over time.) Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org --- doc/src/sgml/ref/create_domain.sgml | 17 ++++++-- src/backend/parser/gram.y | 60 ++++++++++++++++++++++++++-- src/test/regress/expected/domain.out | 8 ++-- src/test/regress/sql/domain.sql | 8 ++-- 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/doc/src/sgml/ref/create_domain.sgml b/doc/src/sgml/ref/create_domain.sgml index 73f9f28d6cf..078bea8410d 100644 --- a/doc/src/sgml/ref/create_domain.sgml +++ b/doc/src/sgml/ref/create_domain.sgml @@ -24,9 +24,9 @@ CREATE DOMAIN name [ AS ] data_type [ COLLATE collation ] [ DEFAULT expression ] - [ constraint [ ... ] ] + [ domain_constraint [ ... ] ] -where constraint is: +where domain_constraint is: [ CONSTRAINT constraint_name ] { NOT NULL | NULL | CHECK (expression) } @@ -190,7 +190,7 @@ Parameters - + Notes @@ -279,6 +279,17 @@ Compatibility The command CREATE DOMAIN conforms to the SQL standard. + + + The syntax NOT NULL in this command is a + PostgreSQL extension. (A standard-conforming + way to write the same would be CHECK (VALUE IS NOT + NULL). However, per , + such constraints a best avoided in practice anyway.) The + NULL constraint is a + PostgreSQL extension (see also ). + diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 0523f7e891e..e8b619926ef 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -524,7 +524,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type generic_set set_rest set_rest_more generic_reset reset_rest SetResetClause FunctionSetResetClause -%type TableElement TypedTableElement ConstraintElem TableFuncElement +%type TableElement TypedTableElement ConstraintElem DomainConstraintElem TableFuncElement %type columnDef columnOptions optionalPeriodName %type def_elem reloption_elem old_aggr_elem operator_def_elem %type def_arg columnElem where_clause where_or_current_clause @@ -596,7 +596,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type col_name_keyword reserved_keyword %type bare_label_keyword -%type TableConstraint TableLikeClause +%type DomainConstraint TableConstraint TableLikeClause %type TableLikeOptionList TableLikeOption %type column_compression opt_column_compression column_storage opt_column_storage %type ColQualList @@ -4334,6 +4334,60 @@ ConstraintElem: } ; +/* + * DomainConstraint is separate from TableConstraint because the syntax for + * NOT NULL constraints is different. For table constraints, we need to + * accept a column name, but for domain constraints, we don't. (We could + * accept something like NOT NULL VALUE, but that seems weird.) CREATE DOMAIN + * (which uses ColQualList) has for a long time accepted NOT NULL without a + * column name, so it makes sense that ALTER DOMAIN (which uses + * DomainConstraint) does as well. None of these syntaxes are per SQL + * standard; we are just living with the bits of inconsistency that have built + * up over time. + */ +DomainConstraint: + CONSTRAINT name DomainConstraintElem + { + Constraint *n = castNode(Constraint, $3); + + n->conname = $2; + n->location = @1; + $$ = (Node *) n; + } + | DomainConstraintElem { $$ = $1; } + ; + +DomainConstraintElem: + CHECK '(' a_expr ')' ConstraintAttributeSpec + { + Constraint *n = makeNode(Constraint); + + n->contype = CONSTR_CHECK; + 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; + $$ = (Node *) n; + } + | NOT NULL_P ConstraintAttributeSpec + { + Constraint *n = makeNode(Constraint); + + n->contype = CONSTR_NOTNULL; + n->location = @1; + n->keys = list_make1(makeString("value")); + /* no NOT VALID support yet */ + processCASbits($3, @3, "NOT NULL", + NULL, NULL, NULL, + &n->is_no_inherit, yyscanner); + n->initially_valid = true; + $$ = (Node *) n; + } + ; + opt_no_inherit: NO INHERIT { $$ = true; } | /* EMPTY */ { $$ = false; } ; @@ -11586,7 +11640,7 @@ AlterDomainStmt: $$ = (Node *) n; } /* ALTER DOMAIN ADD CONSTRAINT ... */ - | ALTER DOMAIN_P any_name ADD_P TableConstraint + | ALTER DOMAIN_P any_name ADD_P DomainConstraint { AlterDomainStmt *n = makeNode(AlterDomainStmt); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index fa8459e10ff..58e0fc53166 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -805,20 +805,20 @@ create table domconnotnulltest , col2 connotnull ); insert into domconnotnulltest default values; -alter domain connotnull add not null value; -- fails +alter domain connotnull add not null; -- fails ERROR: column "col1" of table "domconnotnulltest" contains null values update domconnotnulltest set col1 = 5; -alter domain connotnull add not null value; -- fails +alter domain connotnull add not null; -- fails ERROR: column "col2" of table "domconnotnulltest" contains null values update domconnotnulltest set col2 = 6; -alter domain connotnull add constraint constr1 not null value; +alter domain connotnull add constraint constr1 not null; select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n'; count ------- 1 (1 row) -alter domain connotnull add constraint constr1bis not null value; -- redundant +alter domain connotnull add constraint constr1bis not null; -- redundant select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n'; count ------- diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 763c68f1db6..b9600944fbd 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -477,16 +477,16 @@ ); insert into domconnotnulltest default values; -alter domain connotnull add not null value; -- fails +alter domain connotnull add not null; -- fails update domconnotnulltest set col1 = 5; -alter domain connotnull add not null value; -- fails +alter domain connotnull add not null; -- fails update domconnotnulltest set col2 = 6; -alter domain connotnull add constraint constr1 not null value; +alter domain connotnull add constraint constr1 not null; select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n'; -alter domain connotnull add constraint constr1bis not null value; -- redundant +alter domain connotnull add constraint constr1bis not null; -- redundant select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n'; update domconnotnulltest set col1 = null; -- fails base-commit: bb766cde63b4f624d029b34c9cdd3d0a94fd5b46 -- 2.44.0