Re: ATTACH PARTITION seems to ignore column generation status

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> On Fri, Jan 6, 2023 at 3:53 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I'm not sure to what extent it's sensible for partitions to have
>> GENERATED columns that don't match their parent; but even if that's
>> okay for payload columns I doubt we want to allow partitioning
>> columns to be GENERATED.

> Actually, I'm inclined to disallow partitions to have *any* generated
> columns that are not present in the parent table. The main reason for
> that is the inconvenience of checking that a partition's generated
> columns doesn't override the partition key column of an ancestor that
> is not its immediate parent, which MergeAttributesIntoExisting() has
> access to and would have been locked.

After thinking about this awhile, I feel that we ought to disallow
it in the traditional-inheritance case as well. The reason is that
there are semantic prohibitions on inserting or updating a generated
column, eg

regression=# create table t (f1 int, f2 int generated always as (f1+1) stored);
CREATE TABLE
regression=# update t set f2=42;
ERROR: column "f2" can only be updated to DEFAULT
DETAIL: Column "f2" is a generated column.

It's not very reasonable to have to recheck that for child tables,
and we don't. But if one does this:

regression=# create table pp (f1 int, f2 int);
CREATE TABLE
regression=# create table cc (f1 int, f2 int generated always as (f1+1) stored) inherits(pp);
NOTICE: merging column "f1" with inherited definition
NOTICE: merging column "f2" with inherited definition
CREATE TABLE
regression=# insert into cc values(1);
INSERT 0 1
regression=# update pp set f2 = 99 where f1 = 1;
UPDATE 1
regression=# table cc;
f1 | f2
----+----
1 | 99
(1 row)

That is surely just as broken as the partition-based case.

I also note that the code adjacent to what you added is

/*
* If parent column is generated, child column must be, too.
*/
if (attribute->attgenerated && !childatt->attgenerated)
ereport(ERROR, ...

without any exception for non-partition inheritance, and the
following check for equivalent generation expressions has
no such exception either. So it's not very clear why this
test should have an exception.

> Patch doing it that way is attached. Perhaps the newly added error
> message should match CREATE TABLE .. PARTITION OF's, but I found the
> latter to be not detailed enough, or maybe that's just me.

Maybe we should improve the existing error message while we're at it?

regards, tom lane