Re: ALTER TYPE extensions

Sorry, I missed a bug when we create a typed table using composite
type which has been altered.

postgres=# CREATE TYPE comp_1 AS (x int, y int, z int);
CREATE TYPE
postgres=# ALTER TYPE comp_1 DROP ATTRIBUTE y;
ALTER TYPE
postgres=# CREATE TABLE t1 OF comp_1;
ERROR: cache lookup failed for type 0
postgres=# SELECT attname, attnum, attisdropped FROM pg_attribute
WHERE attrelid = 'comp_1'::regclass;
attname | attnum | attisdropped
------------------------------+--------+--------------
x | 1 | f
........pg.dropped.2........ | 2 | t
z | 3 | f
(3 rows)

Perhaps, we also need to patch at transformOfType() to
skip attributes with attisdropped.

An additional question. It seems me we can remove all the attributes
from the composite type, although CREATE TYPE prohibits to create
a composite type without any attribute.
What does it mean a composite type with no attribute?
Or, do we need a restriction to prevent the last one attribute?

Rest of comments are below.

(2010/09/18 5:44), Peter Eisentraut wrote:
> On fre, 2010-09-17 at 18:15 +0900, KaiGai Kohei wrote:
>> * At the ATPrepAddColumn(), it seems to me someone added a check
>> to prevent adding a new column to typed table, as you try to
>> add in this patch.
>
> Good catch. Redundant checks removed.
>
OK,

>> * At the ATPrepAlterColumnType(), you enclosed an existing code
>> block by "if (tab->relkind == RELKIND_RELATION) { ... }", but
>> it is not indented to appropriate level.
>
> Yeah, just to keep the patch small. ;-)
>
Hmm...
Although I expect the patched routine also should follow the common
coding style in spite of patch size, but it may not be a thing that
I should decide here.
So, I'd like to entrust this decision to committer. OK?

>> * RENAME ATTRIBUTE ... TO ...
>>
>> Even if the composite type to be altered is in use, we can alter
>> the name of attribute. Is it intended?
>
> No. Added a check for it now.
>
OK,

>> BTW, is there any requirement from SQL standard about behavior
>> when we try to add/drop an attribute of composite type in use?
>> This patch always prohibit it, using find_typed_table_dependencies()
>> and find_composite_type_dependencies().
>> However, it seems to me not difficult to alter columns of typed
>> tables subsequent with this ALTER TYPE, although it might be
>> not easy to alter definitions of embedded composite type already
>> in use.
>> Of course, it may be our future works. If so, it's good.
>
> The prohibition on altering types that are used in typed tables is
> actually from the SQL standard. But for now it's just because it's not
> implemented; I plan to work on extending that later.
>
> The restriction by find_composite_type_dependencies() was already there
> for altering tables, and I just kept it the same for now.
>
Thanks for your explanation. It made me clear.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>