0001-WIP-Fix-ALTER-DOMAIN-NOT-NULL-syntax.patch

From 0bdc4944879670b7b53447833fa9f9b385eb0309 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 21 Mar 2024 12:15:11 +0100
Subject: [PATCH] WIP: Fix ALTER DOMAIN NOT NULL syntax

---
 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 39a801a1c38..391a708467e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -522,7 +522,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
 %type <defelt>	def_elem reloption_elem old_aggr_elem operator_def_elem
 %type <node>	def_arg columnElem where_clause where_or_current_clause
@@ -593,7 +593,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
@@ -4251,6 +4251,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; }
 		;
@@ -11493,7 +11547,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 dc58793e3f5..bf5c34bfacf 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 ae1b7fbf97a..ce0c2e3cbc2 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
-- 
2.44.0