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

[KaiGai Kohei]

Is it an expected behavior?

  postgres=# CREATE TABLE t1 (a int, b int);
  CREATE TABLE
  postgres=# CREATE TABLE t2 (b int, c int);
  CREATE TABLE
  postgres=# CREATE TABLE t3 (d int) inherits (t1, t2);
  NOTICE:  merging multiple inherited definitions of column "b"
  CREATE TABLE

The t3.d is inherited from t1 and t2. Its attinhcount is 2.

  postgres=# ALTER TABLE t1 RENAME b TO x;
  ALTER TABLE

It alters name of the column 'b' in the t1 and its child tables ('t3').

  postgres=# SELECT * FROM t1;
   a | x
  ---+---
  (0 rows)

  postgres=# SELECT * FROM t2;
  ERROR:  could not find inherited attribute "b" of relation "t3"

Because t3.b is also inherited from the t2, but ALTER TABLE does not
care about multiple inherited columns well.

I think we should not allow to rename a column with attinhcount > 1.

Any comments?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@...>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[Alvaro Herrera-7]

KaiGai Kohei wrote:

>   postgres=# SELECT * FROM t2;
>   ERROR:  could not find inherited attribute "b" of relation "t3"
>
> Because t3.b is also inherited from the t2, but ALTER TABLE does not
> care about multiple inherited columns well.
>
> I think we should not allow to rename a column with attinhcount > 1.

I think we should fix ALTER TABLE to cope with multiple inheritance.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[Thom Brown]

2009/11/4 Alvaro Herrera <alvherre@...>:

> KaiGai Kohei wrote:
>
>>   postgres=# SELECT * FROM t2;
>>   ERROR:  could not find inherited attribute "b" of relation "t3"
>>
>> Because t3.b is also inherited from the t2, but ALTER TABLE does not
>> care about multiple inherited columns well.
>>
>> I think we should not allow to rename a column with attinhcount > 1.
>
> I think we should fix ALTER TABLE to cope with multiple inheritance.
>

I'd be interested to see how this should work.  Given KaiGai's
example, how would that be resolved?  That column would already be
merged for the inheriting table, so would renaming it somehow unmerge
it?  Given an insertion into t3 would propagate to both column b's in
table t1 and t2, and then renaming b in t1 wouldn't make sense.  Or
would renaming it be prevented due to dependants?  Or should t3's
inheritance of t1 and t2 implicitly bind t1's and t2's column b to one
another so that one affects the other? (i.e. renaming column b in t1
would also rename column b in t2, or both would require renaming
during the same transaction.)

...or something less confusing. :)

Thom

--
Sent via pgsql-hackers mailing list (pgsql-hackers@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[Tom Lane-2]

Thom Brown <thombrown@...> writes:
> 2009/11/4 Alvaro Herrera <alvherre@...>:
>> KaiGai Kohei wrote:
>>> I think we should not allow to rename a column with attinhcount > 1.

>> I think we should fix ALTER TABLE to cope with multiple inheritance.

> I'd be interested to see how this should work.

Yeah.  I don't think a "fix" is possible, because there is no
non-astonishing way for it to behave.  I think KaiGai is right that
forbidding the rename is the best solution.

                        regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[KaiGai Kohei]

Tom Lane wrote:

> Thom Brown <thombrown@...> writes:
>> 2009/11/4 Alvaro Herrera <alvherre@...>:
>>> KaiGai Kohei wrote:
>>>> I think we should not allow to rename a column with attinhcount > 1.
>
>>> I think we should fix ALTER TABLE to cope with multiple inheritance.
>
>> I'd be interested to see how this should work.
>
> Yeah.  I don't think a "fix" is possible, because there is no
> non-astonishing way for it to behave.  I think KaiGai is right that
> forbidding the rename is the best solution.

The attached patch forbids rename when the attribute is inherited
from multiple parents.

  postgres=# CREATE TABLE t1 (a int, b int);
  CREATE TABLE
  postgres=# CREATE TABLE t2 (b int, c int);
  CREATE TABLE
  postgres=# CREATE TABLE t3 (d int) INHERITS (t1, t2);
  NOTICE:  merging multiple inherited definitions of column "b"
  CREATE TABLE
  postgres=# SELECT * FROM t3;
   a | b | c | d
  ---+---+---+---
  (0 rows)

  postgres=# ALTER TABLE t1 RENAME b TO x;
  ERROR:  cannot rename multiple inherited column "b"


The regression test detected a matter in the misc test.

It tries to rename column "a" of "a_star" table, but it failed due to
the new restriction.

  --
  -- test the "star" operators a bit more thoroughly -- this time,
  -- throw in lots of NULL fields...
  --
  -- a is the type root
  -- b and c inherit from a (one-level single inheritance)
  -- d inherits from b and c (two-level multiple inheritance)
  -- e inherits from c (two-level single inheritance)
  -- f inherits from e (three-level single inheritance)
  --
  CREATE TABLE a_star (
      class       char,
      a           int4
  );

  CREATE TABLE b_star (
      b           text
  ) INHERITS (a_star);

  CREATE TABLE c_star (
      c           name
  ) INHERITS (a_star);

  CREATE TABLE d_star (
      d           float8
  ) INHERITS (b_star, c_star);

At the misc test,

  --- 242,278 ----
    ALTER TABLE c_star* RENAME COLUMN c TO cc;
    ALTER TABLE b_star* RENAME COLUMN b TO bb;
    ALTER TABLE a_star* RENAME COLUMN a TO aa;
  + ERROR:  cannot rename multiple inherited column "a"
    SELECT class, aa
       FROM a_star* x
       WHERE aa ISNULL;
  ! ERROR:  column "aa" does not exist
  ! LINE 1: SELECT class, aa
  !

It seems to me it is a case the regression test to be fixed up.
(We don't have any reasonable way to know whether a certain attribute
has a same source, or not.)

Any comments?
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@...>

Index: src/test/regress/sql/inherit.sql
===================================================================
*** src/test/regress/sql/inherit.sql (revision 2388)
--- src/test/regress/sql/inherit.sql (working copy)
*************** CREATE TABLE inh_error1 () INHERITS (t1,
*** 336,338 ****
--- 336,352 ----
  CREATE TABLE inh_error2 (LIKE t4 INCLUDING STORAGE) INHERITS (t1);
 
  DROP TABLE t1, t2, t3, t4, t12_storage, t12_comments, t1_inh, t13_inh, t13_like, t_all;
+
+ -- Test for renaming
+ CREATE TABLE t1 (a int, b int);
+ CREATE TABLE t2 (b int, c int);
+ CREATE TABLE t3 (d int) INHERITS(t1, t2);
+ ALTER TABLE t1 RENAME a TO x;
+ ALTER TABLE t1 RENAME b TO y; -- to be failed
+ ALTER TABLE t3 RENAME d TO z;
+ SELECT * FROM t3;
+ DROP TABLE t3;
+ DROP TABLE t2;
+ DROP TABLE t1;
+
+
Index: src/test/regress/expected/inherit.out
===================================================================
*** src/test/regress/expected/inherit.out (revision 2388)
--- src/test/regress/expected/inherit.out (working copy)
*************** NOTICE:  merging column "a" with inherit
*** 1057,1059 ****
--- 1057,1076 ----
  ERROR:  column "a" has a storage parameter conflict
  DETAIL:  MAIN versus EXTENDED
  DROP TABLE t1, t2, t3, t4, t12_storage, t12_comments, t1_inh, t13_inh, t13_like, t_all;
+ -- Test for renaming
+ CREATE TABLE t1 (a int, b int);
+ CREATE TABLE t2 (b int, c int);
+ CREATE TABLE t3 (d int) INHERITS(t1, t2);
+ NOTICE:  merging multiple inherited definitions of column "b"
+ ALTER TABLE t1 RENAME a TO x;
+ ALTER TABLE t1 RENAME b TO y; -- to be failed
+ ERROR:  cannot rename multiple inherited column "b"
+ ALTER TABLE t3 RENAME d TO z;
+ SELECT * FROM t3;
+  x | b | c | z
+ ---+---+---+---
+ (0 rows)
+
+ DROP TABLE t3;
+ DROP TABLE t2;
+ DROP TABLE t1;
Index: src/backend/commands/tablecmds.c
===================================================================
*** src/backend/commands/tablecmds.c (revision 2388)
--- src/backend/commands/tablecmds.c (working copy)
*************** renameatt(Oid myrelid,
*** 2024,2029 ****
--- 2024,2040 ----
  errmsg("cannot rename inherited column \"%s\"",
  oldattname)));
 
+ /*
+ * If the attribute is inherited from multiple parents, forbid
+ * the renaming, because we don't have any reasonable way to keep
+ * integrity in whole of the inheritance relationship.
+ */
+ if (attform->attinhcount > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ (errmsg("cannot rename multiple inherited column \"%s\"",
+ oldattname))));
+
  /* should not already exist */
  /* this test is deliberately not attisdropped-aware */
  if (SearchSysCacheExists(ATTNAME,


--
Sent via pgsql-hackers mailing list (pgsql-hackers@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers