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 |
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 |