Re: Bugs in CREATE/DROP INDEX CONCURRENTLY

Tom Lane wrote:
> "Kevin Grittner" <kgrittn@mail.com> writes:
>> To put that another way, it should be done at a time when it is
>> sure that no query sees indisvalid = true and no query has yet
>> seen indisready = false. Patch attached. Will apply if nobody sees
>> a problem with it.
>
> The above statement of the requirement doesn't seem to match what
> you put in the comment:
>
>> + * All predicate locks on the index are about to be made invalid. Promote
>> + * them to relation locks on the heap. For correctness this must be done
>> + * after the index was last seen with indisready = true and before it is
>> + * seen with indisvalid = false.

It took me a while to spot it, but yeah -- I reversed the field names
in the comment. :-( The email was right; I fixed the comment.

> and the comment is rather vaguely worded too (last seen by what?).
> Please wordsmith that a bit more.

I tried to leave nothing to the imagination this time.

I think the README or function comments on the predicate locking
should include more about this sort of thing. In retrospect, the
documentation reads more like a maintainer's guide for SSI, and the
user's guide is lacking. Something like that would make it easier for
people working on other features (like DROP INDEX CONCURRENTLY) to
know where to put which calls. That's more than I can do right at the
moment, but I'll make a note of it. I suspect that if enough of this
is in the comment above the function definition, less of it needs to
be near each call site.

-Kevin