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.
> * 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. ;-)
> * 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.
> 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.
New patch attached.