pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
11+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2021-05-14 6a4c071568 0a536004b7 037cc13f41 e5f9821569 Doc: correct erroneous entry in this week's minor release notes.
2021-03-12 8a22977766 e5794cd593 e7f7950f67 f52c5d6749 6c34f186c4 Forbid marking an identity column as nullable.
* BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-04 04:13 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 11+ messages in thread
From: PG Bug reporting form @ 2021-03-04 04:13 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: pavel.boev@invitae.com
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.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-04 14:22 Vik Fearing <vik@postgresfriends.org>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 11+ messages in thread
From: Vik Fearing @ 2021-03-04 14:22 UTC (permalink / raw)
To: pavel.boev@invitae.com; pgsql-bugs@lists.postgresql.org
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
Attachments:
null_identity.diff (text/x-patch, 4.0K) download
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;
+
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-11 14:37 Vik Fearing <vik@postgresfriends.org>
parent: Vik Fearing <vik@postgresfriends.org>
0 siblings, 1 reply; 11+ messages in thread
From: Vik Fearing @ 2021-03-11 14:37 UTC (permalink / raw)
To: pavel.boev@invitae.com; pgsql-bugs@lists.postgresql.org
On 3/4/21 3:22 PM, Vik Fearing wrote:
>
> I can confirm this bug. Attached is a patch to prevent it.
Would anybody like to take a look at this?
--
Vik Fearing
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-12 12:59 Amit Langote <amitlangote09@gmail.com>
parent: Vik Fearing <vik@postgresfriends.org>
0 siblings, 1 reply; 11+ messages in thread
From: Amit Langote @ 2021-03-12 12:59 UTC (permalink / raw)
To: Vik Fearing <vik@postgresfriends.org>; +Cc: pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Mar 11, 2021 at 11:37 PM Vik Fearing <vik@postgresfriends.org> wrote:
> On 3/4/21 3:22 PM, Vik Fearing wrote:
> >
> > I can confirm this bug. Attached is a patch to prevent it.
>
> Would anybody like to take a look at this?
Patch looks reasonable to me.
There seems to be an empty line added at the end, which git diff --check flags:
$ git diff --check
src/test/regress/sql/identity.sql:356: new blank line at EOF.
+-- 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;
+
--
Amit Langote
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-12 15:30 Tom Lane <tgl@sss.pgh.pa.us>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2021-03-12 15:30 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Vik Fearing <vik@postgresfriends.org>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Amit Langote <amitlangote09@gmail.com> writes:
> On Thu, Mar 11, 2021 at 11:37 PM Vik Fearing <vik@postgresfriends.org> wrote:
>> On 3/4/21 3:22 PM, Vik Fearing wrote:
>>> I can confirm this bug. Attached is a patch to prevent it.
>> Would anybody like to take a look at this?
> Patch looks reasonable to me.
I think it's not really necessary to have two different tests; we can
just make the code for IDENTITY include exactly what the code for
CONSTR_NOTNULL does, ie it can set saw_not_null too. The point here
is that IDENTITY can be read as implicitly including NOT NULL, so we
can just act as though that was explicit.
An alternative fix approach is to add to the list of conflict checks
that are made after the loop, but I'm not sure that's any better.
Will push it in a bit.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-03-12 17:21 Vik Fearing <vik@postgresfriends.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Vik Fearing @ 2021-03-12 17:21 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Amit Langote <amitlangote09@gmail.com>; +Cc: pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 3/12/21 4:30 PM, Tom Lane wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
>> On Thu, Mar 11, 2021 at 11:37 PM Vik Fearing <vik@postgresfriends.org> wrote:
>>> On 3/4/21 3:22 PM, Vik Fearing wrote:
>>>> I can confirm this bug. Attached is a patch to prevent it.
>
>>> Would anybody like to take a look at this?
>
>> Patch looks reasonable to me.
>
> Will push it in a bit.
Thanks, Tom.
--
Vik Fearing
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-05-14 20:06 Shay Rojansky <roji@roji.org>
parent: Vik Fearing <vik@postgresfriends.org>
0 siblings, 1 reply; 11+ messages in thread
From: Shay Rojansky @ 2021-05-14 20:06 UTC (permalink / raw)
To: Vik Fearing <vik@postgresfriends.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Amit Langote <amitlangote09@gmail.com>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi all.
I can see the PG 13.3 change note about GENERATED ALWAYS AS IDENTITY no
longer being compatible with an explicit NULL specification. However, it
seems that GENERATED BY DEFAULT AS IDENTITY also is no longer compatible
with null:
CREATE TABLE foo
(
id INTEGER NULL GENERATED BY DEFAULT AS IDENTITY
);
Results in:
ERROR: conflicting NULL/NOT NULL declarations for column "id" of table
"foo"
Is this intended? It seems to make sense to allow NULL to be explicitly
inserted into columns whith as GENERATED BY DEFAULT AS IDENTITY. If this is
intended, maybe the release notes should be updated for this.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-05-14 20:18 Tom Lane <tgl@sss.pgh.pa.us>
parent: Shay Rojansky <roji@roji.org>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2021-05-14 20:18 UTC (permalink / raw)
To: Shay Rojansky <roji@roji.org>; +Cc: Vik Fearing <vik@postgresfriends.org>; Amit Langote <amitlangote09@gmail.com>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Shay Rojansky <roji@roji.org> writes:
> I can see the PG 13.3 change note about GENERATED ALWAYS AS IDENTITY no
> longer being compatible with an explicit NULL specification. However, it
> seems that GENERATED BY DEFAULT AS IDENTITY also is no longer compatible
> with null:
> CREATE TABLE foo
> (
> id INTEGER NULL GENERATED BY DEFAULT AS IDENTITY
> );
> Results in:
> ERROR: conflicting NULL/NOT NULL declarations for column "id" of table
> "foo"
> Is this intended?
GENERATED BY DEFAULT does create a NOT NULL constraint:
regression=# CREATE TABLE foo(id INTEGER GENERATED BY DEFAULT AS IDENTITY);
CREATE TABLE
regression=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+----------------------------------
id | integer | | not null | generated by default as identity
so I think the patch is doing what it was intended to. Whether GENERATED
BY DEFAULT *should* be forcing NOT NULL is a separate question, but
AFAIK it always has.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-05-14 21:18 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2021-05-14 21:18 UTC (permalink / raw)
To: Shay Rojansky <roji@roji.org>; +Cc: Vik Fearing <vik@postgresfriends.org>; Amit Langote <amitlangote09@gmail.com>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
I wrote:
> GENERATED BY DEFAULT does create a NOT NULL constraint:
> ...
> so I think the patch is doing what it was intended to. Whether GENERATED
> BY DEFAULT *should* be forcing NOT NULL is a separate question, but
> AFAIK it always has.
Ah, found it. SQL:2016 11.4 <column definition> syntax rule 16 saith:
If <identity column specification> ICS is specified, then:
...
d) The <column constraint definition> NOT NULL NOT DEFERRABLE is implicit.
The <identity column specification> production includes both cases:
<identity column specification> ::=
GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY
[ <left paren> <common sequence generator options> <right paren> ]
so the spec does clearly say that both alternatives force NOT NULL.
So, it was my error to write the release notes as though only
GENERATED ALWAYS is affected. I'll go adjust that, though
it won't propagate to the website for another three months :-(
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-05-15 15:43 Shay Rojansky <roji@roji.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Shay Rojansky @ 2021-05-15 15:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Vik Fearing <vik@postgresfriends.org>; Amit Langote <amitlangote09@gmail.com>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
> I'll go adjust that
Thanks Tom.
> so the spec does clearly say that both alternatives force NOT NULL.
For what it's worth, it's odd to disallow nullable columns which are also
GENERATED BY DEFAULT AS IDENTITY - unless I'm missing something, it seems
like quite an artificial restriction for a legitimate scenario. After all,
we can do:
CREATE SEQUENCE foo_seq AS integer;
CREATE TABLE foo (
bar INTEGER NULL DEFAULT nextval('foo_seq')
);
... which is logically very similar, and definitely seems useful. Would it
make sense to allow nullable GENERATED BY DEFAULT AS IDENTITY as a PG
extension?
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties
@ 2021-05-15 15:54 Shay Rojansky <roji@roji.org>
parent: Shay Rojansky <roji@roji.org>
0 siblings, 0 replies; 11+ messages in thread
From: Shay Rojansky @ 2021-05-15 15:54 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Vik Fearing <vik@postgresfriends.org>; Amit Langote <amitlangote09@gmail.com>; pavel.boev@invitae.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
> I'll go adjust that
One last pedantic note: the CREATE TABLE page[1] doesn't seem to mention
the syntactic incompatibility between GENERATED ... AS IDENTITY and NULL.
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2021-05-15 15:54 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 04:13 BUG #16913: GENERATED AS IDENTITY column nullability is affected by order of column properties PG Bug reporting form <noreply@postgresql.org>
2021-03-04 14:22 ` Vik Fearing <vik@postgresfriends.org>
2021-03-11 14:37 ` Vik Fearing <vik@postgresfriends.org>
2021-03-12 12:59 ` Amit Langote <amitlangote09@gmail.com>
2021-03-12 15:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-03-12 17:21 ` Vik Fearing <vik@postgresfriends.org>
2021-05-14 20:06 ` Shay Rojansky <roji@roji.org>
2021-05-14 20:18 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-05-14 21:18 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-05-15 15:43 ` Shay Rojansky <roji@roji.org>
2021-05-15 15:54 ` Shay Rojansky <roji@roji.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox