On Sat, Jun 04, 2011 at 05:49:31PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > I could not come up with an actual wrong behavior arising from this usage, so
> > I'll tentatively call it a false positive. reindex_index() could instead
> > unconditionally clear indexInfo->ii_Exclusion* before calling index_build(),
> > then pop pendingReindexedIndexes and call IndexCheckExclusion() itself. Popping
> > pendingReindexedIndexes as we go can make the success of a reindex_relation()
> > dependent on the order in which we choose to rebuild indexes, though.
>
> > Another option is to just remove the assert as not worth preserving.
>
> Removing the assert would be a seriously bad idea IMO. I think we could
> just allow index_build to call ResetReindexProcessing() midstream (ie,
> before it calls IndexCheckExclusion). This does raise the question of
> whether the existing call to IndexCheckExclusion is badly placed and
> we should move it to after the index is "fully" rebuilt. That would
> allow us to avoid doing ResetReindexProcessing until the index is
> clearly safe to use.
>
> So in short, I'm thinking move lines 1760-1772 (in HEAD) of index.c to
> the end of index_build(), then insert a ResetReindexProcessing() call in
> front of them; or maybe only do ResetReindexProcessing there if we
> actually do call IndexCheckExclusion.
Sounds reasonable. Need to remove the index from pendingReindexedIndexes, not
just call ResetReindexProcessing(). Also, wouldn't that specific construction
make the catalog updates fail due to running in the table owner's security
context? But certainly something along those lines will do.
