Re: Support for REINDEX CONCURRENTLY

On Thu, Feb 7, 2013 at 5:15 PM, Andres Freund <span dir="ltr"><<a href="mailto:andres@2ndquadrant.com"
target="_blank">andres@2ndquadrant.com</a>></span>wrote:<br /><div class="gmail_quote"><blockquote
class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 2013-02-07
03:01:36-0500, Tom Lane wrote:<br /> > Andres Freund <<a
href="mailto:andres@2ndquadrant.com">andres@2ndquadrant.com</a>>writes:<br /> > > What about<br /> ><br />
>> 3) Use reltoastidxid if != InvalidOid and manually build the list (using<br /> > > RelationGetIndexList)
otherwise?<br/> ><br /> > Do we actually need reltoastidxid at all?  I always thought having that<br /> >
fieldwas a case of premature optimization.<br /><br /></div>I am a bit doubtful its really measurable as well. Really
supportinga<br /> dynamic number of indexes might be noticeable because we would need to<br /> allocate memory et al
foreach toasted Datum, but only supporting one or<br /> two seems easy enough.<br /><br /> The only advantage besides
thedubious performance advantage of my<br /> proposed solution is that less code needs to change as only<br />
toast_save_datum()would need to change.<br /><div class="im"><br /> > There might be some case<br /> > for
keepingit to avoid breaking any client-side code that might be<br /> > looking at it ... but if you're proposing
changingthe field contents<br /> > anyway, that argument goes right out the window.<br /><br /></div>Well, it would
onlybe 0/InvalidOid while being reindexed concurrently,<br /> but yea.<br /></blockquote>Removing reltoastindxid is
moreappealing for at least 2 reasons regarding current implementation of REINDEX CONCURRENTLY:<br />1) if reltoastidxid
isset to InvalidOid during a concurrent reindex and reindex fails, how would it be possible to set it back to the
correctvalue? This would need more special code, which could become a maintenance burden for sure.<br /> 2) There is
alreadysome special code in my patch to update reltoastidxid to the new Oid value when swapping indexes. Removing that
wouldhonestly make the index swapping cleaner.<br /><br />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
performanceimpact (if any) is welcome if we remove reltoastidxid and use RelationGetIndexList instead.<br /></div>--
<br/>Michael<br />