This patch fixes a regression caused by my recent changes to heap

tuple header.  The fix is based on the thought that HEAP_MOVED_IN is
not needed any more as soon as HEAP_XMIN_COMMITTED has been set.  So
in tqual.c and vacuum.c the HEAP_MOVED bits are cleared when
HEAP_XMIN_COMMITTED is set.

Vacuum robustness is enhanced by rearranging ifs, so that we have a
chance to elog(ERROR, ...) before an assertion fails.

A new regression test is included.

Manfred Koizar

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7e67099e442484f79c691994c7891410d636c899..1809981486f6781430c15b910104bb066b05dabe 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.230 2002/06/20 20:29:27 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.231 2002/07/20 04:57:13 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1533,8 +1533,6 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
 
                        if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
                        {
-                               if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
-                                       elog(ERROR, "Invalid XVAC in tuple header");
                                if (tuple.t_data->t_infomask & HEAP_MOVED_IN)
                                        elog(ERROR, "HEAP_MOVED_IN was not expected");
 
@@ -1545,6 +1543,8 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                                 */
                                if (tuple.t_data->t_infomask & HEAP_MOVED_OFF)
                                {
+                                       if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
+                                               elog(ERROR, "Invalid XVAC in tuple header");
                                        if (keep_tuples == 0)
                                                continue;
                                        if (chain_tuple_moved)          /* some chains was moved
@@ -2008,7 +2008,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
 
                        /*
                         * Mark new tuple as moved_in by vacuum and store vacuum XID
-                        * in t_cmin !!!
+                        * in t_cid !!!
                         */
                        newtup.t_data->t_infomask &=
                                ~(HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID | HEAP_MOVED_OFF);
@@ -2034,7 +2034,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
 
                        /*
                         * Mark old tuple as moved_off by vacuum and store vacuum XID
-                        * in t_cmin !!!
+                        * in t_cid !!!
                         */
                        tuple.t_data->t_infomask &=
                                ~(HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID | HEAP_MOVED_IN);
@@ -2087,12 +2087,12 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                                tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid);
                                if (tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED)
                                        continue;
-                               if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
-                                       elog(ERROR, "Invalid XVAC in tuple header (4)");
                                if (tuple.t_data->t_infomask & HEAP_MOVED_IN)
                                        elog(ERROR, "HEAP_MOVED_IN was not expected (2)");
                                if (tuple.t_data->t_infomask & HEAP_MOVED_OFF)
                                {
+                                       if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
+                                               elog(ERROR, "Invalid XVAC in tuple header (4)");
                                        /* some chains was moved while */
                                        if (chain_tuple_moved)
                                        {                       /* cleaning this page */
@@ -2116,6 +2116,8 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                                                keep_tuples--;
                                        }
                                }
+                               else
+                                       elog(ERROR, "HEAP_MOVED_OFF was expected (2)");
                        }
                }
 
@@ -2225,17 +2227,18 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                        tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid);
                        if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
                        {
+                               if (!(tuple.t_data->t_infomask & HEAP_MOVED))
+                                       elog(ERROR, "HEAP_MOVED_OFF/HEAP_MOVED_IN was expected");
                                if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
                                        elog(ERROR, "Invalid XVAC in tuple header (2)");
                                if (tuple.t_data->t_infomask & HEAP_MOVED_IN)
                                {
                                        tuple.t_data->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple.t_data->t_infomask &= ~HEAP_MOVED;
                                        num_tuples++;
                                }
-                               else if (tuple.t_data->t_infomask & HEAP_MOVED_OFF)
-                                       tuple.t_data->t_infomask |= HEAP_XMIN_INVALID;
                                else
-                                       elog(ERROR, "HEAP_MOVED_OFF/HEAP_MOVED_IN was expected");
+                                       tuple.t_data->t_infomask |= HEAP_XMIN_INVALID;
                        }
                }
                LockBuffer(buf, BUFFER_LOCK_UNLOCK);
@@ -2304,15 +2307,15 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
 
                                if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
                                {
-                                       if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
-                                               elog(ERROR, "Invalid XVAC in tuple header (3)");
                                        if (tuple.t_data->t_infomask & HEAP_MOVED_OFF)
                                        {
+                                               if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
+                                                       elog(ERROR, "Invalid XVAC in tuple header (3)");
                                                itemid->lp_flags &= ~LP_USED;
                                                num_tuples++;
                                        }
                                        else
-                                               elog(ERROR, "HEAP_MOVED_OFF was expected (2)");
+                                               elog(ERROR, "HEAP_MOVED_OFF was expected (3)");
                                }
 
                        }

diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 601208ec259c08576717a21d2870a105ed3e906a..b1302c2740b336b8aa8b90982c5254f6ab4f6000 100644 (file)
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -16,7 +16,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/time/tqual.c,v 1.56 2002/06/20 20:29:41 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/time/tqual.c,v 1.57 2002/07/20 04:57:13 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -92,7 +92,10 @@ HeapTupleSatisfiesItself(HeapTupleHeader tuple)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return false;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -219,6 +222,7 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple)
                                        return false;
                                }
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
                        }
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
@@ -228,7 +232,10 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return false;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -336,6 +343,7 @@ HeapTupleSatisfiesToast(HeapTupleHeader tuple)
                                        return false;
                                }
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
                        }
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
@@ -345,7 +353,10 @@ HeapTupleSatisfiesToast(HeapTupleHeader tuple)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return false;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -389,6 +400,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htuple, CommandId curcid)
                                        return HeapTupleInvisible;
                                }
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
                        }
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
@@ -398,7 +410,10 @@ HeapTupleSatisfiesUpdate(HeapTuple htuple, CommandId curcid)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return HeapTupleInvisible;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -520,6 +535,7 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple)
                                        return false;
                                }
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
                        }
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
@@ -529,7 +545,10 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return false;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -651,6 +670,7 @@ HeapTupleSatisfiesSnapshot(HeapTupleHeader tuple, Snapshot snapshot)
                                        return false;
                                }
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
                        }
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
@@ -660,7 +680,10 @@ HeapTupleSatisfiesSnapshot(HeapTupleHeader tuple, Snapshot snapshot)
                                if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                        return false;
                                if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                               {
                                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                                       tuple->t_infomask &= ~HEAP_MOVED;
+                               }
                                else
                                {
                                        tuple->t_infomask |= HEAP_XMIN_INVALID;
@@ -809,6 +832,7 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin)
                                return HEAPTUPLE_DEAD;
                        }
                        tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                       tuple->t_infomask &= ~HEAP_MOVED;
                }
                else if (tuple->t_infomask & HEAP_MOVED_IN)
                {
@@ -817,7 +841,10 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin)
                        if (TransactionIdIsInProgress(HeapTupleHeaderGetXvac(tuple)))
                                return HEAPTUPLE_INSERT_IN_PROGRESS;
                        if (TransactionIdDidCommit(HeapTupleHeaderGetXvac(tuple)))
+                       {
                                tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+                               tuple->t_infomask &= ~HEAP_MOVED;
+                       }
                        else
                        {
                                tuple->t_infomask |= HEAP_XMIN_INVALID;

diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 919c37e13f65d18e810fbb8b10048eb7309ef6b6..218a5bd14d98ab308549a55f244e72d6b907ec13 100644 (file)
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -38,7 +38,7 @@ test: copy
 # ----------
 # The third group of parallel test
 # ----------
-test: constraints triggers create_misc create_aggregate create_operator create_index inherit
+test: constraints triggers create_misc create_aggregate create_operator create_index inherit vacuum
 
 # Depends on the above
 test: create_view

diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 552d63f9e300750aeba0e13849f261a75fb0a558..3e79a9c307316e048dbd7ca834a8a8253fba68b0 100644 (file)
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -1,4 +1,4 @@
-# $Header: /cvsroot/pgsql/src/test/regress/serial_schedule,v 1.11 2002/07/18 04:43:51 momjian Exp $
+# $Header: /cvsroot/pgsql/src/test/regress/serial_schedule,v 1.12 2002/07/20 04:57:13 momjian Exp $
 # This should probably be in an order similar to parallel_schedule.
 test: boolean
 test: char
@@ -50,6 +50,7 @@ test: create_aggregate
 test: create_operator
 test: create_index
 test: inherit
+test: vacuum
 test: create_view
 test: sanity_check
 test: errors