Subject: Re: BUG #15672: PostgreSQL 11.1/11.2 crashed after dropping a
 partition table
References: <20190426153921.GA22956@alvherre.pgsql>
 <9943.1556305069@sss.pgh.pa.us>
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Message-ID: <3cf270d8-40dc-ea8b-c7b7-4236af6ef972@lab.ntt.co.jp>
Date: Tue, 7 May 2019 17:10:34 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <9943.1556305069@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Amit Langote <amitlangote09@gmail.com>,
        Michael Paquier <michael@paquier.xyz>, jianingy.yang@gmail.com,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
        Pg Hackers <pgsql-hackers@postgresql.org>
Precedence: bulk

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