While testing Alvaro's "cataloguing NOT NULL constraints" patch [1], I
noticed a behavior of inherited column merging during ALTER TABLE that
I thought might be a bug (though see at the bottom).
Note how CREATE TABLE merges the inherited properties of parent foo's
a's NOT NULL into a child bar's own a:
create table foo (a int not null);
create table bar (a int, b int) inherits (foo);
NOTICE: merging column "a" with inherited definition
CREATE TABLE
\d bar
Table "public.bar"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | not null |
b | integer | | |
Inherits: foo
However, ALTER TABLE apparently doesn't pass down the NOT NULL flag
when merging the parent's new column b into a child table's existing
column b:
alter table foo add b int not null;
NOTICE: merging definition of column "b" for child "bar"
\d bar
Table "public.bar"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | not null |
b | integer | | |
Inherits: foo
ATExecAddColumn()'s following block of code that handles the merging
seems inadequate compared to the similar block of MergeAttributes()
called during CREATE TABLE:
/*
* Are we adding the column to a recursion child? If so, check whether to
* merge with an existing definition for the column. If we do merge, we
* must not recurse. Children will already have the column, and recursing
* into them would mess up attinhcount.
*/
if (colDef->inhcount > 0)
{
...
/* Bump the existing child att's inhcount */
childatt->attinhcount++;
CatalogTupleUpdate(attrdesc, &tuple->t_self, tuple);
heap_freetuple(tuple);
/* Inform the user about the merge */
ereport(NOTICE,
(errmsg("merging definition of column \"%s\" for
child \"%s\"",
colDef->colname, RelationGetRelationName(rel))));
table_close(attrdesc, RowExclusiveLock);
return InvalidObjectAddress;
}
This only increments attinhcount of the child's existing column,
unlike MergeAttributes()'s code, which will not only merge the NOT
NULL flag but also check for generated conflicts, so one gets the
following behavior:
create table foo (a int generated always as (1) stored);
create table bar (a int generated always as identity) inherits (foo);
NOTICE: merging column "a" with inherited definition
ERROR: column "a" inherits from generated column but specifies identity
create table bar (a int generated always as (2) stored) inherits (foo);
NOTICE: merging column "a" with inherited definition
ERROR: child column "a" specifies generation expression
HINT: Omit the generation expression in the definition of the child
table column to inherit the generation expression from the parent
table.
create table bar (a int, b int generated always as identity) inherits (foo);
NOTICE: merging column "a" with inherited definition
\d bar
Table "public.bar"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+--------------------------------
a | integer | | | generated always as (1) stored
b | integer | | not null | generated always as identity
Inherits: foo
alter table foo add b int generated always as (1) stored;
NOTICE: merging definition of column "b" for child "bar"
\d bar
Table "public.bar"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+--------------------------------
a | integer | | | generated always as (1) stored
b | integer | | not null | generated always as identity
Inherits: foo
So, adding a column to the parent after-the-fact will allow its
generated definition to differ from that of the child's existing
column, which is not allowed when creating the child table with its
own different generated definition for its column.
I feel like we may have discussed this before and decided that the
$subject is left that way intentionally, but wanted to bring this up
again in the light of Alvaro's patch which touches nearby code.
Should we try to fix this?
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
[1] https://commitfest.postgresql.org/39/3869/
