Thread: pgsql: Comments in IndexBuildHeapScan describe the indexing of
pgsql: Comments in IndexBuildHeapScan describe the indexing of
From
tgl@postgresql.org (Tom Lane)
Date:
Log Message:
-----------
Comments in IndexBuildHeapScan describe the indexing of recently-dead
tuples as needed "to keep VACUUM from complaining", but actually there is
a more compelling reason to do it: failure to do so violates MVCC semantics.
This is because a pre-existing serializable transaction might try to use
the index after we finish (re)building it, and it might fail to find tuples
it should be able to see. We got this mostly right, but not in the case
of partial indexes: the code mistakenly discarded recently-dead tuples for
partial indexes. Fix that, and adjust the comments.
Tags:
----
REL8_1_STABLE
Modified Files:
--------------
pgsql/src/backend/catalog:
index.c (r1.261.2.1 -> r1.261.2.2)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/index.c.diff?r1=1.261.2.1&r2=1.261.2.2)
On Fri, 2006-03-24 at 19:02 -0400, Tom Lane wrote: > Log Message: > ----------- > Comments in IndexBuildHeapScan describe the indexing of recently-dead > tuples as needed "to keep VACUUM from complaining", but actually there is > a more compelling reason to do it: failure to do so violates MVCC semantics. > This is because a pre-existing serializable transaction might try to use > the index after we finish (re)building it, and it might fail to find tuples > it should be able to see. We got this mostly right, but not in the case > of partial indexes: the code mistakenly discarded recently-dead tuples for > partial indexes. Fix that, and adjust the comments. > > Tags: > ---- > REL8_1_STABLE > > Modified Files: > -------------- > pgsql/src/backend/catalog: > index.c (r1.261.2.1 -> r1.261.2.2) > (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/index.c.diff?r1=1.261.2.1&r2=1.261.2.2) Well spotted... I notice the same error occurs in REL8_0_STABLE, REL7_4_STABLE and REL7_3_STABLE. This is a data loss bug, so why not back apply to those releases also? Best Regards, Simon Riggs
Simon Riggs <simon@2ndquadrant.com> writes:
> On Fri, 2006-03-24 at 19:02 -0400, Tom Lane wrote:
>> Comments in IndexBuildHeapScan describe the indexing of recently-dead
>> tuples as needed "to keep VACUUM from complaining", but actually there is
>> a more compelling reason to do it: failure to do so violates MVCC semantics.
> I notice the same error occurs in REL8_0_STABLE, REL7_4_STABLE and
> REL7_3_STABLE. This is a data loss bug, so why not back apply to those
> releases also?
I'm not sure it really qualifies as "data loss", since the answers would
be only transiently wrong. I chose not to back-patch further than 8.1
for a couple of reasons:
* It's a corner case, and given the lack of complaints, the risk of
breaking something in the back branches has to be factored into the
decision. I believe that the testing I did in HEAD validates the
patch well enough against 8.1, but the further back you go the less
well the correlation applies.
* The same problem exists with respect to CLUSTER, not to mention the
table-rewriting variants of ALTER TABLE. Any patch for CLUSTER will
be far more invasive and is unlikely to get back-patched at all.
regards, tom lane