On 21.03.24 12:23, Peter Eisentraut wrote:
All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".
Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. As long as you are only dealing with CHECK
constraints, there is no difference, but it shows up when using NOT NULL
constraint syntax. I agree that this is unsatisfactory. Attached is a
patch to try to sort this out.
After studying this a bit more, I think moving forward in this direction
is the best way. Attached is a new patch version, mainly with a more
elaborate commit message. This patch makes the not-null constraint
syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes
the respective documentation correct.
(Note that, as I show in the commit message, commit e5da0fe3c22 had in
passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just
reverting that commit wouldn't be a complete solution.)
From 4e4de402dda81798bb0286a09d39c6ed2f087228 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
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 <replaceable class="parameter">name</replaceable> [ AS ]
<replaceable class="parameter">data_type</replaceable>
[ COLLATE <replaceable>collation</replaceable> ]
[ DEFAULT <replaceable>expression</replaceable> ]
- [ <replaceable class="parameter">constraint</replaceable> [ ... ] ]
+ [ <replaceable class="parameter">domain_constraint</replaceable> [ ... ] ]
-<phrase>where <replaceable class="parameter">constraint</replaceable>
is:</phrase>
+<phrase>where <replaceable class="parameter">domain_constraint</replaceable>
is:</phrase>
[ CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> ]
{ NOT NULL | NULL | CHECK (<replaceable
class="parameter">expression</replaceable>) }
@@ -190,7 +190,7 @@ <title>Parameters</title>
</variablelist>
</refsect1>
- <refsect1>
+ <refsect1 id="sql-createdomain-notes">
<title>Notes</title>
<para>
@@ -279,6 +279,17 @@ <title>Compatibility</title>
The command <command>CREATE DOMAIN</command> conforms to the SQL
standard.
</para>
+
+ <para>
+ The syntax <literal>NOT NULL</literal> in this command is a
+ <productname>PostgreSQL</productname> extension. (A standard-conforming
+ way to write the same would be <literal>CHECK (VALUE IS NOT
+ NULL)</literal>. However, per <xref linkend="sql-createdomain-notes"/>,
+ such constraints a best avoided in practice anyway.) The
+ <literal>NULL</literal> <quote>constraint</quote> is a
+ <productname>PostgreSQL</productname> extension (see also <xref
+ linkend="sql-createtable-compatibility"/>).
+ </para>
</refsect1>
<refsect1 id="sql-createdomain-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 <vsetstmt> generic_set set_rest set_rest_more generic_reset reset_rest
SetResetClause FunctionSetResetClause
-%type <node> TableElement TypedTableElement ConstraintElem TableFuncElement
+%type <node> TableElement TypedTableElement ConstraintElem
DomainConstraintElem TableFuncElement
%type <node> columnDef columnOptions optionalPeriodName
%type <defelt> def_elem reloption_elem old_aggr_elem operator_def_elem
%type <node> def_arg columnElem where_clause where_or_current_clause
@@ -596,7 +596,7 @@ static Node *makeRecursiveViewSelect(char *relname, List
*aliases, Node *query);
%type <keyword> col_name_keyword reserved_keyword
%type <keyword> bare_label_keyword
-%type <node> TableConstraint TableLikeClause
+%type <node> DomainConstraint TableConstraint TableLikeClause
%type <ival> TableLikeOptionList TableLikeOption
%type <str> column_compression opt_column_compression
column_storage opt_column_storage
%type <list> 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 <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
