Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f4e787c

Browse files
committed
Fix bugs in contrib/pg_visibility.
collect_corrupt_items() failed to initialize tuple.t_self. While HeapTupleSatisfiesVacuum() doesn't actually use that value, it does Assert that it's valid, so that the code would dump core if ip_posid chanced to be zero. (That's somewhat unlikely, which probably explains how this got missed. In any case it wouldn't matter for field use.) Also, collect_corrupt_items was returning the wrong TIDs, that is the contents of t_ctid rather than the tuple's own location. This would be the same thing in simple cases, but it could be wrong if, for example, a past update attempt had been rolled back, leaving a live tuple whose t_ctid doesn't point at itself. Also, in pg_visibility(), guard against trying to read a page past the end of the rel. The VM code handles inquiries beyond the end of the map by silently returning zeroes, and it seems like we should do the same thing here. I ran into the assertion failure while using pg_visibility to check pg_upgrade's behavior, and then noted the other problems while reading the code. Report: <29043.1475288648@sss.pgh.pa.us>
1 parent b01f9ed commit f4e787c

File tree

1 file changed

+26
-12
lines changed

1 file changed

+26
-12
lines changed

contrib/pg_visibility/pg_visibility.c

+26-12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* pg_visibility.c
44
* display visibility map information and page-level visibility bits
55
*
6+
* Copyright (c) 2016, PostgreSQL Global Development Group
7+
*
68
* contrib/pg_visibility/pg_visibility.c
79
*-------------------------------------------------------------------------
810
*/
@@ -54,6 +56,10 @@ static bool tuple_all_visible(HeapTuple tup, TransactionId OldestXmin,
5456

5557
/*
5658
* Visibility map information for a single block of a relation.
59+
*
60+
* Note: the VM code will silently return zeroes for pages past the end
61+
* of the map, so we allow probes up to MaxBlockNumber regardless of the
62+
* actual relation size.
5763
*/
5864
Datum
5965
pg_visibility_map(PG_FUNCTION_ARGS)
@@ -122,13 +128,22 @@ pg_visibility(PG_FUNCTION_ARGS)
122128
values[0] = BoolGetDatum((mapbits & VISIBILITYMAP_ALL_VISIBLE) != 0);
123129
values[1] = BoolGetDatum((mapbits & VISIBILITYMAP_ALL_FROZEN) != 0);
124130

125-
buffer = ReadBuffer(rel, blkno);
126-
LockBuffer(buffer, BUFFER_LOCK_SHARE);
131+
/* Here we have to explicitly check rel size ... */
132+
if (blkno < RelationGetNumberOfBlocks(rel))
133+
{
134+
buffer = ReadBuffer(rel, blkno);
135+
LockBuffer(buffer, BUFFER_LOCK_SHARE);
127136

128-
page = BufferGetPage(buffer);
129-
values[2] = BoolGetDatum(PageIsAllVisible(page));
137+
page = BufferGetPage(buffer);
138+
values[2] = BoolGetDatum(PageIsAllVisible(page));
130139

131-
UnlockReleaseBuffer(buffer);
140+
UnlockReleaseBuffer(buffer);
141+
}
142+
else
143+
{
144+
/* As with the vismap, silently return 0 for pages past EOF */
145+
values[2] = BoolGetDatum(false);
146+
}
132147

133148
relation_close(rel, AccessShareLock);
134149

@@ -611,14 +626,13 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
611626
/* Dead line pointers are neither all-visible nor frozen. */
612627
if (ItemIdIsDead(itemid))
613628
{
614-
ItemPointerData tid;
615-
616-
ItemPointerSet(&tid, blkno, offnum);
617-
record_corrupt_item(items, &tid);
629+
ItemPointerSet(&(tuple.t_self), blkno, offnum);
630+
record_corrupt_item(items, &tuple.t_self);
618631
continue;
619632
}
620633

621634
/* Initialize a HeapTupleData structure for checks below. */
635+
ItemPointerSet(&(tuple.t_self), blkno, offnum);
622636
tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid);
623637
tuple.t_len = ItemIdGetLength(itemid);
624638
tuple.t_tableOid = relid;
@@ -649,12 +663,12 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
649663
RecomputedOldestXmin = GetOldestXmin(NULL, true);
650664

651665
if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin))
652-
record_corrupt_item(items, &tuple.t_data->t_ctid);
666+
record_corrupt_item(items, &tuple.t_self);
653667
else
654668
{
655669
OldestXmin = RecomputedOldestXmin;
656670
if (!tuple_all_visible(&tuple, OldestXmin, buffer))
657-
record_corrupt_item(items, &tuple.t_data->t_ctid);
671+
record_corrupt_item(items, &tuple.t_self);
658672
}
659673
}
660674

@@ -665,7 +679,7 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
665679
if (check_frozen)
666680
{
667681
if (heap_tuple_needs_eventual_freeze(tuple.t_data))
668-
record_corrupt_item(items, &tuple.t_data->t_ctid);
682+
record_corrupt_item(items, &tuple.t_self);
669683
}
670684
}
671685

0 commit comments

Comments
 (0)