Skip site navigation (1) Skip section navigation (2)

Re: Fix for regression caused by heap tuple header changes

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Manfred Koizar <mkoi-pg(at)aon(dot)at>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Fix for regression caused by heap tuple header changes
Date: 2002-07-18 20:36:56
Message-ID: 200207182036.g6IKauK14987@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-patches
Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


Manfred Koizar wrote:
> 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.
> 
> Servus
>  Manfred

> diff -ruN ../base/src/backend/commands/vacuum.c src/backend/commands/vacuum.c
> --- ../base/src/backend/commands/vacuum.c	2002-07-15 20:39:19.000000000 +0200
> +++ src/backend/commands/vacuum.c	2002-07-16 13:41:59.000000000 +0200
> @@ -1534,8 +1534,6 @@
>  
>  			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");
>  
> @@ -1546,6 +1544,8 @@
>  				 */
>  				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
> @@ -2009,7 +2009,7 @@
>  
>  			/*
>  			 * 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);
> @@ -2035,7 +2035,7 @@
>  
>  			/*
>  			 * 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);
> @@ -2088,12 +2088,12 @@
>  				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 */
> @@ -2117,6 +2117,8 @@
>  						keep_tuples--;
>  					}
>  				}
> +				else
> +					elog(ERROR, "HEAP_MOVED_OFF was expected (2)");
>  			}
>  		}
>  
> @@ -2226,17 +2228,18 @@
>  			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);
> @@ -2305,15 +2308,15 @@
>  
>  				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 -ruN ../base/src/backend/utils/time/tqual.c src/backend/utils/time/tqual.c
> --- ../base/src/backend/utils/time/tqual.c	2002-06-21 02:12:23.000000000 +0200
> +++ src/backend/utils/time/tqual.c	2002-07-16 13:19:59.000000000 +0200
> @@ -92,7 +92,10 @@
>  				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 @@
>  					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 @@
>  				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 @@
>  					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 @@
>  				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 @@
>  					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 @@
>  				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 @@
>  					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 @@
>  				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 @@
>  					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 @@
>  				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 @@
>  				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 @@
>  			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 -ruN ../base/src/test/regress/expected/vacuum.out src/test/regress/expected/vacuum.out
> --- ../base/src/test/regress/expected/vacuum.out	2002-07-16 14:12:56.000000000 +0200
> +++ src/test/regress/expected/vacuum.out	2002-07-16 14:26:05.000000000 +0200
> @@ -0,0 +1,59 @@
> +--
> +-- VACUUM
> +--
> +CREATE TABLE vactst (i INT);
> +INSERT INTO vactst VALUES (1);
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst VALUES (0);
> +SELECT count(*) FROM vactst;
> + count 
> +-------
> +  2049
> +(1 row)
> +
> +DELETE FROM vactst WHERE i != 0;
> +SELECT * FROM vactst;
> + i 
> +---
> + 0
> +(1 row)
> +
> +VACUUM FULL vactst;
> +UPDATE vactst SET i = i + 1;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst VALUES (0);
> +SELECT count(*) FROM vactst;
> + count 
> +-------
> +  2049
> +(1 row)
> +
> +DELETE FROM vactst WHERE i != 0;
> +VACUUM FULL vactst;
> +DELETE FROM vactst;
> +SELECT * FROM vactst;
> + i 
> +---
> +(0 rows)
> +
> +DROP TABLE vactst;
> diff -ruN ../base/src/test/regress/parallel_schedule src/test/regress/parallel_schedule
> --- ../base/src/test/regress/parallel_schedule	2002-07-10 23:29:32.000000000 +0200
> +++ src/test/regress/parallel_schedule	2002-07-16 14:01:23.000000000 +0200
> @@ -38,7 +38,7 @@
>  # ----------
>  # 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 -ruN ../base/src/test/regress/serial_schedule src/test/regress/serial_schedule
> --- ../base/src/test/regress/serial_schedule	2002-07-10 23:31:16.000000000 +0200
> +++ src/test/regress/serial_schedule	2002-07-16 14:01:45.000000000 +0200
> @@ -50,6 +50,7 @@
>  test: create_operator
>  test: create_index
>  test: inherit
> +test: vacuum
>  test: create_view
>  test: sanity_check
>  test: errors
> diff -ruN ../base/src/test/regress/sql/vacuum.sql src/test/regress/sql/vacuum.sql
> --- ../base/src/test/regress/sql/vacuum.sql	2002-07-16 14:36:30.000000000 +0200
> +++ src/test/regress/sql/vacuum.sql	2002-07-16 14:22:43.000000000 +0200
> @@ -0,0 +1,42 @@
> +--
> +-- VACUUM
> +--
> +
> +CREATE TABLE vactst (i INT);
> +INSERT INTO vactst VALUES (1);
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst VALUES (0);
> +SELECT count(*) FROM vactst;
> +DELETE FROM vactst WHERE i != 0;
> +SELECT * FROM vactst;
> +VACUUM FULL vactst;
> +UPDATE vactst SET i = i + 1;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst SELECT * FROM vactst;
> +INSERT INTO vactst VALUES (0);
> +SELECT count(*) FROM vactst;
> +DELETE FROM vactst WHERE i != 0;
> +VACUUM FULL vactst;
> +DELETE FROM vactst;
> +SELECT * FROM vactst;
> +
> +DROP TABLE vactst;

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

In response to

pgsql-patches by date

Next:From: Tom LaneDate: 2002-07-18 20:40:18
Subject: Re: Between Node
Previous:From: Bruce MomjianDate: 2002-07-18 20:36:24
Subject: Re: minor GEQO fixes

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group