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

On 2012-11-27 14:41:34 -0500, Tom Lane wrote:
> 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.

Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY
... USING someindex ; is done? Also I think indoption might be written
to as well.

> >> 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.

Oh, sorry, misunderstood you.

>
> >> 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.

Good point.

> 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.

indislive seems better than that, yes.

> >>> 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.

Oh, I only was talking about the inplace changes. The DROP INDEX
CONCURRENTLY breakage definitely needs to get backpatched.

Greetings,

Andres Freund

--Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services