Re: BUG #15672: PostgreSQL 11.1/11.2 crashed after dropping a partition table

On 2019/04/27 3:57, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
>> Um, this one doesn't apply because of yesterday's 87259588d0ab.
>
> Before we spend too much time on minutiae, we should ask ourselves whether
> this patch is even going in the right direction. I'm not sure.
>
> One point is that if we simply adopt the old index as-is, we won't see
> any updates in its metadata. An example is that if we have an index
> on a varchar(10) column, and we alter the column to varchar(12),
> the current behavior is to generate a new index that agrees with that:
>
> regression=# create table pp(f1 varchar(10) unique);
> CREATE TABLE
> regression=# \d pp_f1_key
> Index "public.pp_f1_key"
> Column | Type | Key? | Definition
> --------+-----------------------+------+------------
> f1 | character varying(10) | yes | f1
> unique, btree, for table "public.pp"
>
> regression=# alter table pp alter column f1 type varchar(12);
> ALTER TABLE
> regression=# \d pp_f1_key
> Index "public.pp_f1_key"
> Column | Type | Key? | Definition
> --------+-----------------------+------+------------
> f1 | character varying(12) | yes | f1
> unique, btree, for table "public.pp"
>
> With this patch, I believe, the index column will still claim to be
> varchar(10).

You're right, that's what happens.

> Is that OK? It might not actually break anything
> right now, but at the very least it's likely to be confusing.
> Also, it'd essentially render the declared types/typmods of index
> columns untrustworthy, which seems like something that would come
> back to bite us.

That's definitely misleading.

Still, I think it'd be nice if we didn't have to do full-blown
DefineIndex() in this case if only to update the pg_attribute tuples of
the index relation. Maybe we could update them directly in the ALTER
COLUMN TYPE's code path?

Thanks,
Amit