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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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 Content-Type Size
pgsql-fix-inherit-attype.2.patch application/octect-stream 15.6 KB

In response to

Responses

Browse pgsql-hackers by date

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