On 30.06.23 13:44, Alvaro Herrera wrote:
OK, so here's a new attempt to get this working correctly.
Attached is a small fixup patch for the documentation.
Furthermore, there are a few outdated comments that are probably left
over from previous versions of this patch set.
* src/backend/catalog/pg_constraint.c
Outdated comment:
+ /* only tuples for CHECK constraints should be given */
+ Assert(((Form_pg_constraint) GETSTRUCT(constrTup))->contype ==
CONSTRAINT_NOTNULL);
* src/backend/parser/gram.y
Should processCASbits() set &n->skip_validation, like in the CHECK
case? _outConstraint() looks at the field, so it seems relevant.
* src/backend/parser/parse_utilcmd.c
The updated comment says
List *ckconstraints; /* CHECK and NOT NULL constraints */
but it seems to me that NOT NULL constraints are not actually added
there but in nnconstraints instead.
It would be nice if the field nnconstraints was listed after
ckconstraints consistently throughout the file. It's a bit random
right now.
This comment is outdated:
+ /*
+ * For NOT NULL declarations, we need to mark the column as
+ * not nullable, and set things up to have a CHECK
constraint
+ * created. Also, duplicate NOT NULL declarations are not
+ * allowed.
+ */
About this:
case CONSTR_CHECK:
cxt->ckconstraints = lappend(cxt->ckconstraints,
constraint);
+
+ /*
+ * XXX If the user says CHECK (IS NOT NULL), should we turn
+ * that into a regular NOT NULL constraint?
+ */
break;
I think this was decided against.
+ /*
+ * Copy NOT NULL constraints, too (these do not require any option
to have
+ * been given).
+ */
Shouldn't that be governed by the INCLUDING CONSTRAINTS option?
Btw., there is some asymmetry here between check constraints and
not-null constraints: Check constraints are in the tuple descriptor,
but not-null constraints are not. Should that be unified? Or at
least explained?
* src/include/nodes/parsenodes.h
This comment appears to be outdated:
+ * intermixed in tableElts, and constraints and notnullcols are NIL. After
+ * parse analysis, tableElts contains just ColumnDefs, notnullcols has been
+ * filled with not-nullable column names from various sources, and
constraints
+ * contains just Constraint nodes (in fact, only CONSTR_CHECK nodes, in the
+ * present implementation).
There is no "notnullcolls".
* src/test/regress/parallel_schedule
This change appears to be correct, but unrelated to this patch, so I
suggest committing this separately.
* src/test/regress/sql/create_table.sql
-SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =
'part_b'::regclass;
+SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =
'part_b'::regclass ORDER BY coninhcount DESC, conname;
Maybe add conname to the select list here as well, for consistency with
nearby queries.
From c481b58c3e48ff0ab4738e5cf2440d2ad6fc3e47 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 3 Jul 2023 15:44:59 +0200
Subject: [PATCH] fixup! Add pg_constraint rows for NOT NULL constraints
---
doc/src/sgml/catalogs.sgml | 2 +-
doc/src/sgml/ref/alter_table.sgml | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9137b1bc58..36460126f4 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2552,7 +2552,7 @@ <title><structname>pg_constraint</structname>
Columns</title>
<para>
<literal>c</literal> = check constraint,
<literal>f</literal> = foreign key constraint,
- <literal>n</literal> = not null constraint,
+ <literal>n</literal> = not-null constraint,
<literal>p</literal> = primary key constraint,
<literal>u</literal> = unique constraint,
<literal>t</literal> = constraint trigger,
diff --git a/doc/src/sgml/ref/alter_table.sgml
b/doc/src/sgml/ref/alter_table.sgml
index 0b65731b1f..2c4138e4e9 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -113,13 +113,12 @@
[ CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> ]
{ CHECK ( <replaceable class="parameter">expression</replaceable> ) [ NO
INHERIT ] |
+ NOT NULL <replaceable class="parameter">column_name</replaceable> [ NO
INHERIT ] |
UNIQUE [ NULLS [ NOT ] DISTINCT ] ( <replaceable
class="parameter">column_name</replaceable> [, ... ] ) <replaceable
class="parameter">index_parameters</replaceable> |
PRIMARY KEY ( <replaceable class="parameter">column_name</replaceable> [,
... ] ) <replaceable class="parameter">index_parameters</replaceable> |
EXCLUDE [ USING <replaceable class="parameter">index_method</replaceable> ]
( <replaceable class="parameter">exclude_element</replaceable> WITH
<replaceable class="parameter">operator</replaceable> [, ... ] ) <replaceable
class="parameter">index_parameters</replaceable> [ WHERE ( <replaceable
class="parameter">predicate</replaceable> ) ] |
FOREIGN KEY ( <replaceable class="parameter">column_name</replaceable> [,
... ] ) REFERENCES <replaceable class="parameter">reftable</replaceable> [ (
<replaceable class="parameter">refcolumn</replaceable> [, ... ] ) ]
- [ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE <replaceable
class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable
class="parameter">referential_action</replaceable> ] |
- NOT NULL <replaceable class="parameter">column_name</replaceable> [ NO
INHERIT ]
-}
+ [ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE <replaceable
class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable
class="parameter">referential_action</replaceable> ] }
[ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
<phrase>and <replaceable
class="parameter">table_constraint_using_index</replaceable> is:</phrase>
@@ -1765,7 +1764,7 @@ <title>Examples</title>
<title>Compatibility</title>
<para>
- The forms <literal>ADD COLUMN</literal>
+ The forms <literal>ADD [COLUMN]</literal>,
<literal>DROP [COLUMN]</literal>, <literal>DROP IDENTITY</literal>,
<literal>RESTART</literal>,
<literal>SET DEFAULT</literal>, <literal>SET DATA TYPE</literal> (without
<literal>USING</literal>),
<literal>SET GENERATED</literal>, and <literal>SET
<replaceable>sequence_option</replaceable></literal>
@@ -1773,7 +1772,7 @@ <title>Compatibility</title>
The form <literal>ADD <replaceable>table_constraint</replaceable></literal>
conforms with the SQL standard when the <literal>USING INDEX</literal> and
<literal>NOT VALID</literal> clauses are omitted and the constraint type is
- one of <literal>UNIQUE</literal>, <literal>PRIMARY KEY</literal>
+ one of <literal>CHECK</literal>, <literal>UNIQUE</literal>,
<literal>PRIMARY KEY</literal>,
or <literal>REFERENCES</literal>.
The other forms are
<productname>PostgreSQL</productname> extensions of the SQL standard.
--
2.41.0
