From 9895b35cb88edc30b836661dbc26d7665716b5a0 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 15 Apr 2024 08:20:34 +0200 Subject: [PATCH] Fix ALTER DOMAIN NOT NULL syntax This addresses a few problems with commit e5da0fe3c22 ("Catalog domain not-null constraints"). 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.) In passing, this also changes the psql \dD output to not show not-null constraints in the column "Check", since it's already shown in the column "Nullable". This has also been off since e5da0fe3c22. Reviewed-by: jian he 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/backend/utils/adt/ruleutils.c | 2 +- src/bin/psql/describe.c | 2 +- src/test/regress/expected/domain.out | 22 ++++++++-- src/test/regress/sql/domain.sql | 12 ++++-- 6 files changed, 99 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/ref/create_domain.sgml b/doc/src/sgml/ref/create_domain.sgml index 73f9f28d6c..ce55520348 100644 --- a/doc/src/sgml/ref/create_domain.sgml +++ b/doc/src/sgml/ref/create_domain.sgml @@ -24,9 +24,9 @@ PostgreSQL documentation 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 @@ CREATE DOMAIN name [ AS ] - + Notes @@ -279,6 +279,17 @@ CREATE TABLE us_snail_addy ( 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 are 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 0523f7e891..e8b619926e 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/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 24e3514b00..b3428e2ae4 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2523,7 +2523,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, else if (conForm->contypid) { /* conkey is null for domain not-null constraints */ - appendStringInfoString(&buf, "NOT NULL VALUE"); + appendStringInfoString(&buf, "NOT NULL"); } break; } diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 6433497bcd..1f0a056d90 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -4449,7 +4449,7 @@ listDomains(const char *pattern, bool verbose, bool showSystem) " CASE WHEN t.typnotnull THEN 'not null' END as \"%s\",\n" " t.typdefault as \"%s\",\n" " pg_catalog.array_to_string(ARRAY(\n" - " SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM pg_catalog.pg_constraint r WHERE t.oid = r.contypid\n" + " SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM pg_catalog.pg_constraint r WHERE t.oid = r.contypid AND r.contype = 'c'\n" " ), ' ') as \"%s\"", gettext_noop("Schema"), gettext_noop("Name"), diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index fa8459e10f..db0b8a180a 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -785,6 +785,13 @@ alter domain con add constraint t check (VALUE < 1); -- fails ERROR: column "col1" of table "domcontest" contains values that violate the new constraint alter domain con add constraint t check (VALUE < 34); alter domain con add check (VALUE > 0); +\dD con + List of domains + Schema | Name | Type | Collation | Nullable | Default | Check +--------+------+---------+-----------+----------+---------+-------------------------------------- + public | con | integer | | | | CHECK (VALUE < 34) CHECK (VALUE > 0) +(1 row) + insert into domcontest values (-5); -- fails ERROR: value for domain con violates check constraint "con_check" insert into domcontest values (42); -- fails @@ -805,26 +812,33 @@ 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 ------- 1 (1 row) +\dD connotnull + List of domains + Schema | Name | Type | Collation | Nullable | Default | Check +--------+------------+---------+-----------+----------+---------+------- + public | connotnull | integer | | not null | | +(1 row) + update domconnotnulltest set col1 = null; -- fails ERROR: domain connotnull does not allow null values alter domain connotnull drop constraint constr1; diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 763c68f1db..b5a70ee8be 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -458,6 +458,8 @@ alter domain con add constraint t check (VALUE < 1); -- fails alter domain con add constraint t check (VALUE < 34); alter domain con add check (VALUE > 0); +\dD con + insert into domcontest values (-5); -- fails insert into domcontest values (42); -- fails insert into domcontest values (5); @@ -477,18 +479,20 @@ create table domconnotnulltest ); 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'; +\dD connotnull + update domconnotnulltest set col1 = null; -- fails alter domain connotnull drop constraint constr1;