Re: [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Thom Brown <thombrown(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
Date: 2010-01-14 03:43:27
Message-ID: 4B4E92DF.1020409@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The similar matter can be reproduced with ALTER TABLE ... TYPE statement,
not only RENAME TO option.

postgres=# CREATE TABLE t1 (a int);
CREATE TABLE
postgres=# CREATE TABLE s1 (a int);
CREATE TABLE

postgres=# CREATE TABLE ts (b int) inherits (t1, s1);
NOTICE: merging multiple inherited definitions of column "a"
CREATE TABLE
postgres=# ALTER TABLE t1 ALTER a TYPE text;
ALTER TABLE

postgres=# insert into t1 values ('aaa');
INSERT 0 1
postgres=# insert into ts values ('bbb', 2);
INSERT 0 1
postgres=# SELECT * FROM t1;
a
-----
aaa
bbb
(2 rows)

postgres=# SELECT * FROM s1;
ERROR: attribute "a" of relation "ts" does not match parent's type

In the renameatt(), we can count an expected inhcount of the column to be
renamed on find_all_inheritors() at the top-level recursion.
But it does not work well for the new one, because it is handled within
the common ATPrepCmd() scheme.

I reconsidered that we need a function to check whether the given column
is inherited from multiple root parents, or not, for each levels.
Then, it can be called from both of renameatt() and ATPrepAlterColumnType().

(2010/01/04 18:55), KaiGai Kohei wrote:
>>> The method I suggested would allow the
>>> necessary information to be extracted during the initial search for
>>> child tables, which we have to do anyway.
>>
>> find_all_inheritors() returns a clean list which does not contain
>> duplicated OID of the inherited relation, so it seems to me we need
>> to change the function prototype but it affects other parts, or to add
>> a new function which also returns number of duplications, not only OIDs.
>>
>> Or, we can call find_inheritance_children() in renameatt() as if
>> find_all_inheritors() doing except for list_concat_unique_oid().
>
> The attached patch modified the condition to prevent renaming.
>
> It computes an expected inhcount for each child relations on the initial
> call of the renameatt() for the parent relation.
> The find_all_inheritors_with_inhcount() returns OID of the inherited
> relations and the expected inhcoundt. If a child relation has diamond
> inheritance tree, it has its expected inhcount larger than 1.
>
> This patch raises an error, if pg_attribute.inhcount is larger than
> the expected inhcount. It can be happen when the attribute to be
> renamed is merged from any other unrelated relations in the child
> relations.
>
> See the example:
>
> postgres=# CREATE TABLE t1 (a int);
> CREATE TABLE
> postgres=# CREATE TABLE t2 (b int) inherits (t1);
> CREATE TABLE
> postgres=# CREATE TABLE t3 (c int) inherits (t1);
> CREATE TABLE
> postgres=# CREATE TABLE t4 (d int) inherits (t2, t3);
> NOTICE: merging multiple inherited definitions of column "a"
> CREATE TABLE
>
> postgres=# ALTER TABLE t1 RENAME a TO x;
> ALTER TABLE
> postgres=# \d t4
> Table "public.t4"
> Column | Type | Modifiers
> --------+---------+-----------
> x | integer |
> b | integer |
> c | integer |
> d | integer |
> Inherits: t2,
> t3
>
> We can rename a of t1, t2, t3 and t4 correctly, although t4.a has inherited from
> multiple relations.
>
> postgres=# CREATE TABLE s1 (x int);
> CREATE TABLE
> postgres=# CREATE TABLE t5 (e int) inherits (t2, t3, s1);
> NOTICE: merging multiple inherited definitions of column "x"
> NOTICE: merging multiple inherited definitions of column "x"
> CREATE TABLE
> postgres=# ALTER TABLE t1 RENAME x TO y;
> ERROR: cannot rename multiple inherited column "x"
>
> But, the new t5 prevent to rename "x" to "y", because t5.x is also inherited from
> the s1 and merged. So, its inhcount is 3 larger than expected inhcount (=2).
>
> postgres=# SELECT attname, attinhcount FROM pg_attribute where attname='x' and attrelid='t5'::regclass;
> attname | attinhcount
> ---------+-------------
> x | 3
> (1 row)
>
> Thanks,
>
>
>
>

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Takahiro Itagaki 2010-01-14 03:57:39 Re: improving log management
Previous Message Tom Lane 2010-01-14 03:37:58 Re: improving log management