Skip to content

Commit 39b66a9

Browse files
committed
Fix COPY FREEZE with CLOBBER_CACHE_ALWAYS
This adds code omitted from commit 7db0cd2 by accident, which had two consequences. Firstly, only rows inserted by heap_multi_insert were frozen as expected when running COPY FREEZE, while heap_insert left rows unfrozen. That however includes rows in TOAST tables, so a lot of data might have been left unfrozen. Secondly, page might have been left partially empty after relcache invalidation. This addresses both of those issues. Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com
1 parent 183bbd1 commit 39b66a9

File tree

2 files changed

+88
-12
lines changed

2 files changed

+88
-12
lines changed

src/backend/access/heap/heapam.c

+77-1
Original file line numberDiff line numberDiff line change
@@ -1880,8 +1880,12 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
18801880
TransactionId xid = GetCurrentTransactionId();
18811881
HeapTuple heaptup;
18821882
Buffer buffer;
1883+
Page page = NULL;
18831884
Buffer vmbuffer = InvalidBuffer;
1885+
bool starting_with_empty_page;
18841886
bool all_visible_cleared = false;
1887+
bool all_frozen_set = false;
1888+
uint8 vmstatus = 0;
18851889

18861890
/*
18871891
* Fill in tuple header fields and toast the tuple if necessary.
@@ -1894,11 +1898,36 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
18941898
/*
18951899
* Find buffer to insert this tuple into. If the page is all visible,
18961900
* this will also pin the requisite visibility map page.
1901+
*
1902+
* Also pin visibility map page if COPY FREEZE inserts tuples into an
1903+
* empty page. See all_frozen_set below.
18971904
*/
18981905
buffer = RelationGetBufferForTuple(relation, heaptup->t_len,
18991906
InvalidBuffer, options, bistate,
19001907
&vmbuffer, NULL);
19011908

1909+
1910+
/*
1911+
* If we're inserting frozen entry into an empty page,
1912+
* set visibility map bits and PageAllVisible() hint.
1913+
*
1914+
* If we're inserting frozen entry into already all_frozen page,
1915+
* preserve this state.
1916+
*/
1917+
if (options & HEAP_INSERT_FROZEN)
1918+
{
1919+
page = BufferGetPage(buffer);
1920+
1921+
starting_with_empty_page = PageGetMaxOffsetNumber(page) == 0;
1922+
1923+
if (visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer))
1924+
vmstatus = visibilitymap_get_status(relation,
1925+
BufferGetBlockNumber(buffer), &vmbuffer);
1926+
1927+
if ((starting_with_empty_page || vmstatus & VISIBILITYMAP_ALL_FROZEN))
1928+
all_frozen_set = true;
1929+
}
1930+
19021931
/*
19031932
* We're about to do the actual insert -- but check for conflict first, to
19041933
* avoid possibly having to roll back work we've just done.
@@ -1922,14 +1951,28 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
19221951
RelationPutHeapTuple(relation, buffer, heaptup,
19231952
(options & HEAP_INSERT_SPECULATIVE) != 0);
19241953

1925-
if (PageIsAllVisible(BufferGetPage(buffer)))
1954+
/*
1955+
* If the page is all visible, need to clear that, unless we're only
1956+
* going to add further frozen rows to it.
1957+
*
1958+
* If we're only adding already frozen rows to a page that was empty or
1959+
* marked as all visible, mark it as all-visible.
1960+
*/
1961+
if (PageIsAllVisible(BufferGetPage(buffer)) && !(options & HEAP_INSERT_FROZEN))
19261962
{
19271963
all_visible_cleared = true;
19281964
PageClearAllVisible(BufferGetPage(buffer));
19291965
visibilitymap_clear(relation,
19301966
ItemPointerGetBlockNumber(&(heaptup->t_self)),
19311967
vmbuffer, VISIBILITYMAP_VALID_BITS);
19321968
}
1969+
else if (all_frozen_set)
1970+
{
1971+
/* We only ever set all_frozen_set after reading the page. */
1972+
Assert(page);
1973+
1974+
PageSetAllVisible(page);
1975+
}
19331976

19341977
/*
19351978
* XXX Should we set PageSetPrunable on this page ?
@@ -1977,6 +2020,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
19772020
xlrec.flags = 0;
19782021
if (all_visible_cleared)
19792022
xlrec.flags |= XLH_INSERT_ALL_VISIBLE_CLEARED;
2023+
if (all_frozen_set)
2024+
xlrec.flags = XLH_INSERT_ALL_FROZEN_SET;
19802025
if (options & HEAP_INSERT_SPECULATIVE)
19812026
xlrec.flags |= XLH_INSERT_IS_SPECULATIVE;
19822027
Assert(ItemPointerGetBlockNumber(&heaptup->t_self) == BufferGetBlockNumber(buffer));
@@ -2025,6 +2070,29 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
20252070

20262071
END_CRIT_SECTION();
20272072

2073+
/*
2074+
* If we've frozen everything on the page, update the visibilitymap.
2075+
* We're already holding pin on the vmbuffer.
2076+
*
2077+
* No need to update the visibilitymap if it had all_frozen bit set
2078+
* before this insertion.
2079+
*/
2080+
if (all_frozen_set && ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0))
2081+
{
2082+
Assert(PageIsAllVisible(page));
2083+
Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer));
2084+
2085+
/*
2086+
* It's fine to use InvalidTransactionId here - this is only used
2087+
* when HEAP_INSERT_FROZEN is specified, which intentionally
2088+
* violates visibility rules.
2089+
*/
2090+
visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
2091+
InvalidXLogRecPtr, vmbuffer,
2092+
InvalidTransactionId,
2093+
VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
2094+
}
2095+
20282096
UnlockReleaseBuffer(buffer);
20292097
if (vmbuffer != InvalidBuffer)
20302098
ReleaseBuffer(vmbuffer);
@@ -8708,6 +8776,10 @@ heap_xlog_insert(XLogReaderState *record)
87088776
ItemPointerSetBlockNumber(&target_tid, blkno);
87098777
ItemPointerSetOffsetNumber(&target_tid, xlrec->offnum);
87108778

8779+
/* check that the mutually exclusive flags are not both set */
8780+
Assert (!((xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED) &&
8781+
(xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)));
8782+
87118783
/*
87128784
* The visibility map may need to be fixed even if the heap page is
87138785
* already up-to-date.
@@ -8925,6 +8997,10 @@ heap_xlog_multi_insert(XLogReaderState *record)
89258997
if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
89268998
PageClearAllVisible(page);
89278999

9000+
/* XLH_INSERT_ALL_FROZEN_SET implies that all tuples are visible */
9001+
if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)
9002+
PageSetAllVisible(page);
9003+
89289004
MarkBufferDirty(buffer);
89299005
}
89309006
if (BufferIsValid(buffer))

src/backend/access/heap/hio.c

+11-11
Original file line numberDiff line numberDiff line change
@@ -396,19 +396,19 @@ RelationGetBufferForTuple(Relation relation, Size len,
396396
* target.
397397
*/
398398
targetBlock = GetPageWithFreeSpace(relation, len + saveFreeSpace);
399+
}
399400

400-
/*
401-
* If the FSM knows nothing of the rel, try the last page before we
402-
* give up and extend. This avoids one-tuple-per-page syndrome during
403-
* bootstrapping or in a recently-started system.
404-
*/
405-
if (targetBlock == InvalidBlockNumber)
406-
{
407-
BlockNumber nblocks = RelationGetNumberOfBlocks(relation);
401+
/*
402+
* If the FSM knows nothing of the rel, try the last page before we
403+
* give up and extend. This avoids one-tuple-per-page syndrome during
404+
* bootstrapping or in a recently-started system.
405+
*/
406+
if (targetBlock == InvalidBlockNumber)
407+
{
408+
BlockNumber nblocks = RelationGetNumberOfBlocks(relation);
408409

409-
if (nblocks > 0)
410-
targetBlock = nblocks - 1;
411-
}
410+
if (nblocks > 0)
411+
targetBlock = nblocks - 1;
412412
}
413413

414414
loop:

0 commit comments

Comments
 (0)