(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@ak.jp.nec.com>