Re: Bugs in CREATE/DROP INDEX CONCURRENTLY - Mailing list pgsql-hackers

Andres Freund <andres@2ndquadrant.com> writes:
> On 2012-11-27 13:45:08 -0500, Tom Lane wrote:
>> In short, all flag changes in pg_index should be done by
>> update-in-place, and the tuple's xmin will never change from the
>> original creating transaction (until frozen).

> Hm. That doesn't sound that easy to guarantee. Several ALTER TABLE and
> ALTER INDEX operations are expected to work transactionally right
> now. As far as I see there is nothing that prohibits a indexcheckxmin
> requiring index to be altered while indexcheckxmin is still required?

I said "in pg_index".  There is no reason to ever alter an index's
pg_index entry transactionally, because we don't support redefining
the index columns.  The stuff you are allowed to ALTER is pretty much
irrelevant to the index's life as an index.

>> What we want the xmin to do, for indcheckxmin purposes, is reflect the
>> time at which the index started to be included in HOT-safety decisions.
>> Since we're never going to move the xmin, that means that *we cannot
>> allow REINDEX to mark a dead index live again*.

> That would be a regression compared with the current state though. We
> have officially documented REINDEX as a way out of INVALID indexes...

It's a way out of failed CREATE operations.  If DROP fails at the last
step, you won't be able to go back, but why would you want to?  Just
do the DROP again.

>> Anybody feel like bikeshedding the flag column name?  I'm thinking
>> "indislive" but maybe there's a better name.

> I personally would slightly favor indisdead instead...

Meh ... the other two flags are positive, in the sense of
true-is-the-normal-state, so I thought this one should be too.

I had also toyed with "indishot", to reflect the idea that this controls
whether the index is included in HOT-safety decisions, but that seems
maybe a little too cute.

>>> Btw, even if we manage to find a sensible fix for this I would suggest
>>> postponing it after the next back branch release.

>> AFAICS this is a data loss/corruption problem, and as such a "must fix".
>> If we can't have it done by next week, I'd rather postpone the releases
>> until it is done.

> Ok, just seemed like a rather complex fix in a short time for something
> that seemingly hasn't been noticed since 8.3. I am a bit worried about
> introducing something worse while fixing this.

Hm?  The fact that the DROP patch broke CREATE INDEX CONCURRENTLY is a
new and very nasty bug in 9.2.  I would agree with you if we were
considering the unsafe-row-update problem alone, but it seems like we
might as well fix both aspects while we're looking at this code.

There is a question of whether we should risk trying to back-patch the
in-place-update changes further than 9.2.  Given the lack of complaints
I'm inclined not to, but could be persuaded differently.
        regards, tom lane