Re: Support for REINDEX CONCURRENTLY

On Thu, Feb 7, 2013 at 5:15 PM, Andres Freund <andres(at)2ndquadrant(dot)com>wrote:

> On 2013-02-07 03:01:36 -0500, Tom Lane wrote:
> > Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > > What about
> >
> > > 3) Use reltoastidxid if != InvalidOid and manually build the list
> (using
> > > RelationGetIndexList) otherwise?
> >
> > Do we actually need reltoastidxid at all? I always thought having that
> > field was a case of premature optimization.
>
> I am a bit doubtful its really measurable as well. Really supporting a
> dynamic number of indexes might be noticeable because we would need to
> allocate memory et al for each toasted Datum, but only supporting one or
> two seems easy enough.
>
> The only advantage besides the dubious performance advantage of my
> proposed solution is that less code needs to change as only
> toast_save_datum() would need to change.
>
> > There might be some case
> > for keeping it to avoid breaking any client-side code that might be
> > looking at it ... but if you're proposing changing the field contents
> > anyway, that argument goes right out the window.
>
> Well, it would only be 0/InvalidOid while being reindexed concurrently,
> but yea.
>
Removing reltoastindxid is more appealing for at least 2 reasons regarding
current implementation of REINDEX CONCURRENTLY:
1) if reltoastidxid is set to InvalidOid during a concurrent reindex and
reindex fails, how would it be possible to set it back to the correct
value? This would need more special code, which could become a maintenance
burden for sure.
2) There is already some special code in my patch to update reltoastidxid
to the new Oid value when swapping indexes. Removing that would honestly
make the index swapping cleaner.

Btw, I think that if this optimization for toast relations is done, it
should be a separate patch. Also, as I am not a specialist in toast
indexes, any opinion about potential performance impact (if any) is welcome
if we remove reltoastidxid and use RelationGetIndexList instead.
--
Michael