Re: Allowing ALTER TYPE to change storage strategy

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allowing ALTER TYPE to change storage strategy
Date: 2020-02-28 18:59:49
Message-ID: 4872.1582916389@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> My understanding is that pg_type.typstorage essentially specifies two
> things: (a) default storage strategy for the attributes with this type,
> and (b) whether the type implementation is prepared to handle TOAST-ed
> values or not. And pg_attribute.attstorage has to respect this - when
> the type says PLAIN then the attribute can't simply use strategy that
> would enable TOAST.

Check.

> Luckily, this is only a problem when switching typstorage to PLAIN (i.e.
> when disabling TOAST for the type). The attached v1 patch checks if
> there are attributes with non-PLAIN storage for this type, and errors
> out if it finds one. But unfortunately that's not entirely correct,
> because ALTER TABLE only sets storage for new data. A table may be
> created with e.g. EXTENDED storage for an attribute, a bunch of rows may
> be loaded and then the storage for the attribute may be changed to
> PLAIN. That would pass the check as it's currently in the patch, yet
> there may be TOAST-ed values for the type with PLAIN storage :-(

> I'm not entirely sure what to do about this, but I think it's OK to just
> reject changes in this direction (from non-PLAIN to PLAIN storage).

Yeah, I think you should just reject that: once toast-capable, always
toast-capable. Scanning pg_attribute is entirely insufficient because
of race conditions --- and while we accept such races in some other
places, this seems like a place where the risk is too high and the
value too low.

Anybody who really needs to go in that direction still has the alternative
of manually frobbing the catalogs (and taking the responsibility for
races and un-toasting whatever's stored already).

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2020-02-28 20:38:55 Re: Add LogicalTapeSetExtend() to logtape.c
Previous Message Tom Lane 2020-02-28 18:45:29 Re: Assert failure due to "drop schema pg_temp_3 cascade" for temporary tables and \d+ is not showing any info after drooping temp table schema