Re: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash

I wrote:
> PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
>> When executing the following queries:
>> create table pt (a int, b int) partition by list (b);
>> create table t(a pt, check (a = '(1, 2)'::pt));
>> alter table pt alter column a type char(4);
>> \d+ t
>> The server crashes with the following stack:

> Hmm. We really ought to reject the ALTER TABLE. We do if "pt"
> is a plain table:

So the problem is that ATRewriteTables is supposed to make this check
(by calling find_composite_type_dependencies), but first it does

/* Relations without storage may be ignored here */
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;

so the test is skipped for a partitioned table. There is a separate
check in ATPrepAlterColumnType:

if (tab->relkind == RELKIND_COMPOSITE_TYPE ||
tab->relkind == RELKIND_FOREIGN_TABLE)
{
/*
* For composite types and foreign tables, do this check now. Regular
* tables will check it later when the table is being rewritten.
*/
find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL);
}

The best fix seems to be to make that check use the inverse condition.
I also experimented with moving the "Relations without storage" early
exit down past the find_composite_type_dependencies step, in hopes of
getting rid of the check in ATPrepAlterColumnType. But that fails to
cover all cases, because we might not set the rewrite flag.

> I think we're also failing to worry about the rowtype of the
> constant in t's check constraint; it seems like that has to
> be complained of as well, even if the underyling columns
> aren't of type "pt".

This seems to be all right, but I thought it'd be prudent to add
a test case covering it.

regards, tom lane