Re: BUG #16045: vacuum_db crash and illegal memory alloc after pg_upgrade from PG11 to PG12

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Sun, Oct 13, 2019 at 02:26:48PM -0400, Tom Lane wrote:
>> Might be a good idea to exclude attisdropped columns in the part of the
>> recursive query that's looking for sql_identifier columns of composite
>> types.  I'm not sure if composites can have dropped columns today,

[ I checked this, they can ]

>> but even if they can't it seems like a wise bit of future-proofing.
>> (We'll no doubt have occasion to use this logic again...)

> Hmm? How could that be safe? Let's say we have a composite type with a
> sql_identifier column, it's used in a table with data, and we drop the
> column. We need the pg_type information to parse the existing, so how
> could we skip attisdropped columns?

It works exactly like it does for table rowtypes.

regression=# create type cfoo as (f1 int, f2 int, f3 int);
CREATE TYPE
regression=# alter type cfoo drop attribute f2;
ALTER TYPE
regression=# select attname,atttypid,attisdropped,attlen,attalign from pg_attribute where attrelid = 'cfoo'::regclass;
           attname            | atttypid | attisdropped | attlen | attalign
------------------------------+----------+--------------+--------+----------
 f1                           |       23 | f            |      4 | i
 ........pg.dropped.2........ |        0 | t            |      4 | i
 f3                           |       23 | f            |      4 | i
(3 rows)

All we need to skip over the dead data is attlen/attalign, which are
preserved in pg_attribute even if the pg_type row is gone.

As this example shows, you don't really *have* to check attisdropped
because atttypid will be set to zero.  But the latter is just a
defense measure in case somebody forgets to check attisdropped;
you're not supposed to forget that.

            regards, tom lane