On 2019/04/27 3:57, Tom Lane wrote: > Alvaro Herrera <alvhe...@2ndquadrant.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