Re: ALTER TABLE ADD COLUMN fast default

Hi,

On 2018-02-20 13:50:54 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2018-02-20 13:07:30 -0500, Tom Lane wrote:
> >> I can easily think of problems that will ensue if we try to support that
> >> case, because right now the toast mechanisms assume that OOL toasted
> >> values can only be referenced from the associated table.
>
> > What problem are you seeing with treating the toasted value to be from
> > pg_attribute? I'm only drinking my first coffee just now, so I might be
> > missing something...
>
> Locking/vacuuming is exactly what I'm worried about. Right now, a
> transaction cannot reference a toast value without holding at least
> AccessShare on the parent table.

Is that actually reliably true? Doesn't e.g. use a variable from a
rolled-back subtransaction in plpgsql circumvent this already? There
are a few bugs around this though, so that's not really a convincing
argument of there not being any danger :/

> That would not be true of OOL fast defaults that are living in
> pg_attribute's toast table (if it had one). If you think this isn't
> potentially problematic, see the problems that forced us into hacks
> like 08e261cbc.

I don't think the equivalent problem quite applies here, the OOL default
should afaics be immutable. But yea, it's a concern.

> I guess the fix equivalent to 08e261cbc would be to require any toasted
> fast-default value to be detoasted into line whenever it's copied into
> a tupledesc, rather than possibly being fetched later.

Yea, thats probably a good idea. Correctness aside, it's probably better
to detoast once rather than do so for every single row for performance
reasons (detoasting the same row over and over in multiple backends
would lead to quite the contention on used buffer headers and relation
locks).

> > Now we certainly would need to make sure that the corresponding
> > pg_attribute row containing the default value doesn't go away too early,
> > but that shouldn't be much of a problem given that we never remove
> > them. I wondered for a second if there's problematic cases where the
> > default value is referenced by an index, and then the default-adding
> > transaction rolls back. But I can't construct anything realistically
> > problematic.
>
> Oooh ... but no, because we don't allow toasted values to be copied into
> indexes, for (I think) exactly this reason. See index_form_tuple.

I don't think that'd necessarily provide full protection - what about a
index recheck during a lossy bitmap index scan for example? But given
that any such index creation would also be rolled back I'm not too
concerned.

Greetings,

Andres Freund