public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Amit Langote <[email protected]>
Cc: [email protected]
Subject: Re: ATTACH PARTITION seems to ignore column generation status
Date: Fri, 06 Jan 2023 12:32:52 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+HiwqEDfKy6znR_AHvH8CYgjxSo19rX3DH-3e0rgb0qsuGW_w@mail.gmail.com>
References: <[email protected]>
	<CA+HiwqEDfKy6znR_AHvH8CYgjxSo19rX3DH-3e0rgb0qsuGW_w@mail.gmail.com>

Amit Langote <[email protected]> writes:
> On Fri, Jan 6, 2023 at 3:53 AM Tom Lane <[email protected]> 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

view thread (4+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: ATTACH PARTITION seems to ignore column generation status
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox