Fix for regression caused by heap tuple header changes
| От | Manfred Koizar |
|---|---|
| Тема | Fix for regression caused by heap tuple header changes |
| Дата | |
| Msg-id | fn88juoppeach5avalao7ial0v41hns5h4@4ax.com обсуждение исходный текст |
| Ответы |
Re: Fix for regression caused by heap tuple header changes
(Bruce Momjian <pgman@candle.pha.pa.us>)
Re: Fix for regression caused by heap tuple header changes (Bruce Momjian <pgman@candle.pha.pa.us>) |
| Список | pgsql-patches |
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;
В списке pgsql-patches по дате отправления: