Subject: Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected
 by order of column properties
To: pavel.boev@invitae.com, pgsql-bugs@lists.postgresql.org
References: <16913-3b5198410f67d8c6@postgresql.org>
From: Vik Fearing <vik@postgresfriends.org>
Message-ID: <7c02b352-a2e4-e71a-ae69-7e410daf5774@postgresfriends.org>
Date: Thu, 4 Mar 2021 15:22:55 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <16913-3b5198410f67d8c6@postgresql.org>
Content-Type: multipart/mixed;
 boundary="------------08F4650CF64541126D6FC2EF"
Content-Language: en-US
Archived-At: 
 <https://www.postgresql.org/message-id/7c02b352-a2e4-e71a-ae69-7e410daf5774%40postgresfriends.org>
Precedence: bulk

This is a multi-part message in MIME format.
--------------08F4650CF64541126D6FC2EF
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

On 3/4/21 5:13 AM, PG Bug reporting form wrote:
> The following bug has been logged on the website:
> 
> Bug reference:      16913
> Logged by:          Pavel Boev
> Email address:      pavel.boev@invitae.com
> PostgreSQL version: 13.2
> Operating system:   Debian 8.3.0-6
> Description:        
> 
> I noticed that nullability of columns having GENERATED AS IDENTITY option is
> affected by the order of NULL option, so the next query will create a
> "my_table" table having non-nullable column "generated":
> 
> create table my_table (
>   generated integer null generated by default as identity,
>   data text not null
> );
> 
> Whereas the following query will create a "my_table" table having nullable
> column "generated":
> 
> create table my_table (
>   generated integer generated by default as identity null,
>   data text not null
> );
> 
> I noticed the issue appeared in postgres 13.2, however folks say that they
> bug affects versions 10, 11 and 12 as well.

I can confirm this bug.  Attached is a patch to prevent it.
-- 
Vik Fearing

--------------08F4650CF64541126D6FC2EF
Content-Type: text/x-patch; charset=UTF-8;
 name="null_identity.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="null_identity.diff"

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 3b2b227683..8585b1abf2 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -840,6 +840,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       column</firstterm>.  It will have an implicit sequence attached to it
       and the column in new rows will automatically have values from the
       sequence assigned to it.
+      Such a column is always <literal>NOT NULL</literal>.
      </para>
 
      <para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 75266caeb4..f16021a2e2 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -657,6 +657,13 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 									column->colname, cxt->relation->relname),
 							 parser_errposition(cxt->pstate,
 												constraint->location)));
+				if (saw_identity)
+					ereport(ERROR,
+							(errcode(ERRCODE_SYNTAX_ERROR),
+							 errmsg("identity column \"%s\" of table \"%s\" must be NOT NULL",
+									column->colname, cxt->relation->relname),
+							 parser_errposition(cxt->pstate,
+												constraint->location)));
 				column->is_not_null = false;
 				saw_nullable = true;
 				break;
@@ -700,6 +707,14 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 								 errmsg("identity columns are not supported on partitions")));
 
+					if (saw_nullable && !column->is_not_null)
+						ereport(ERROR,
+								(errcode(ERRCODE_SYNTAX_ERROR),
+								 errmsg("identity column \"%s\" of table \"%s\" must be NOT NULL",
+										column->colname, cxt->relation->relname),
+								 parser_errposition(cxt->pstate,
+													constraint->location)));
+
 					ctype = typenameType(cxt->pstate, column->typeName, NULL);
 					typeOid = ((Form_pg_type) GETSTRUCT(ctype))->oid;
 					ReleaseSysCache(ctype);
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index fbca0333a2..83e7d3a971 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -547,3 +547,14 @@ CREATE TABLE itest14 (id serial);
 ALTER TABLE itest14 ALTER id DROP DEFAULT;
 ALTER TABLE itest14 ALTER id ADD GENERATED BY DEFAULT AS IDENTITY;
 INSERT INTO itest14 (id) VALUES (DEFAULT);
+-- Identity columns must be NOT NULL (cf bug #16913)
+CREATE TABLE itest15 (id integer GENERATED ALWAYS AS IDENTITY NULL); -- fail
+ERROR:  identity column "id" of table "itest15" must be NOT NULL
+LINE 1: ...ABLE itest15 (id integer GENERATED ALWAYS AS IDENTITY NULL);
+                                                                 ^
+CREATE TABLE itest15 (id integer NULL GENERATED ALWAYS AS IDENTITY); -- fail
+ERROR:  identity column "id" of table "itest15" must be NOT NULL
+LINE 1: CREATE TABLE itest15 (id integer NULL GENERATED ALWAYS AS ID...
+                                              ^
+CREATE TABLE itest15 (id integer GENERATED ALWAYS AS IDENTITY NOT NULL); DROP TABLE itest15;
+CREATE TABLE itest15 (id integer NOT NULL GENERATED ALWAYS AS IDENTITY); DROP TABLE itest15;
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index cdda186720..48aab70c06 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -346,3 +346,11 @@ CREATE TABLE itest14 (id serial);
 ALTER TABLE itest14 ALTER id DROP DEFAULT;
 ALTER TABLE itest14 ALTER id ADD GENERATED BY DEFAULT AS IDENTITY;
 INSERT INTO itest14 (id) VALUES (DEFAULT);
+
+-- Identity columns must be NOT NULL (cf bug #16913)
+
+CREATE TABLE itest15 (id integer GENERATED ALWAYS AS IDENTITY NULL); -- fail
+CREATE TABLE itest15 (id integer NULL GENERATED ALWAYS AS IDENTITY); -- fail
+CREATE TABLE itest15 (id integer GENERATED ALWAYS AS IDENTITY NOT NULL); DROP TABLE itest15;
+CREATE TABLE itest15 (id integer NOT NULL GENERATED ALWAYS AS IDENTITY); DROP TABLE itest15;
+

--------------08F4650CF64541126D6FC2EF--