Skip site navigation (1) Skip section navigation (2)

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: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, Bernd Helmle <mailings(at)oopsware(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-02-02 01:47:17
Message-ID: 4B678425.8020101@ak.jp.nec.com (view raw or flat)
Thread:
Lists: pgsql-hackers
(2010/02/02 9:48), KaiGai Kohei wrote:
>>> Also, the ALTER COLUMN TYPE case should be also fixed in the 9.1 release
>>> (or 9.0.1?).
>>
>> If the fix is something we could commit for 9.0.1, then we ought to do
>> it now before 9.0 is released.  If you want to submit a follow-on
>> patch to address ALTER COLUMN TYPE once this is committed, then please
>> do so.
> 
> The attached patch also fixes ALTER COLUMN TYPE case.
> 
> It replaced the 'recursing' argument in ATPrepCmd() by 'expected_parents',
> and it is delivered to ATPrepAlterColumnType(). The logic to forbid altering
> the column type is same as renameatt().
> ATSimpleRecursion() is also modified to use forboth() to call ATPrepCmd()
> recursively.
> 
> One concern is at ATOneLevelRecursion() which is called by ATPrepAddColumn()
> only, and it also calls ATPrepCmd() for the direct children.
> Right now, I set 1 on the 'expected_parents'. However, IMO, here is no reason
> why we cannot rewrite the ATPrepAddColumn() using ATSimpleRecursion().
> 
> Eventually, ATExecAddColumn() shall be invoked several times for same column,
> if the inheritance tree has diamond-inheritance structure. And, it increments
> pg_attribute.attinhcount except for the first invocation.
> If we store the 'expected_parents' on the ColumnDef->inhcount, we don't need
> to call the ATExecAddColumn() more than once in a single ALTER TABLE command.
> 
> Any comments? And, when should we do it? 9.0? 9.1?

The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.

There are two regression test fails, because it does not call ATExecAddColumn()
twice or more in diamond-inheritance cases, so it does not notice merging
definitions of columns.

If we should go on right now, I'll add and fix regression tests, and submit
a formal patch again. If not, I'll work it later.

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

Attachment: pgsql-fix-inherit-attype.2.patch
Description: application/octect-stream (15.6 KB)

In response to

Responses

pgsql-hackers by date

Next:From: KaiGai KoheiDate: 2010-02-02 01:55:41
Subject: Re: Largeobject Access Controls (r2460)
Previous:From: Robert HaasDate: 2010-02-02 01:44:51
Subject: Re: remove contrib/xml2

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group