Revert most of 39b66a91bd

Reverts most of commit 39b66a91bd, which was found to cause significant
regression for REFRESH MATERIALIZED VIEW. This means only rows inserted
by heap_multi_insert will benefit from the optimization, implemented in
commit 7db0cd2145.

Reported-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoA%3D%3Df2VSw3c-Cp_y%3DWLKHMKc1D6s7g3YWsCOvgaYPpJcg%40mail.gmail.com

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index bd60129aeb7af55d5e39e210d2460d2ffba0ac67..2433998f39bdb8170f20b3a8b5c650c7ae351646 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2063,12 +2063,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    TransactionId xid = GetCurrentTransactionId();
    HeapTuple   heaptup;
    Buffer      buffer;
-   Page        page = NULL;
    Buffer      vmbuffer = InvalidBuffer;
-   bool        starting_with_empty_page;
    bool        all_visible_cleared = false;
-   bool        all_frozen_set = false;
-   uint8       vmstatus = 0;
 
    /* Cheap, simplistic check that the tuple matches the rel's rowtype. */
    Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
@@ -2085,36 +2081,11 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    /*
     * Find buffer to insert this tuple into.  If the page is all visible,
     * this will also pin the requisite visibility map page.
-    *
-    * Also pin visibility map page if COPY FREEZE inserts tuples into an
-    * empty page. See all_frozen_set below.
     */
    buffer = RelationGetBufferForTuple(relation, heaptup->t_len,
                                       InvalidBuffer, options, bistate,
                                       &vmbuffer, NULL);
 
-
-   /*
-    * If we're inserting frozen entry into an empty page, set visibility map
-    * bits and PageAllVisible() hint.
-    *
-    * If we're inserting frozen entry into already all_frozen page, preserve
-    * this state.
-    */
-   if (options & HEAP_INSERT_FROZEN)
-   {
-       page = BufferGetPage(buffer);
-
-       starting_with_empty_page = PageGetMaxOffsetNumber(page) == 0;
-
-       if (visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer))
-           vmstatus = visibilitymap_get_status(relation,
-                                               BufferGetBlockNumber(buffer), &vmbuffer);
-
-       if ((starting_with_empty_page || vmstatus & VISIBILITYMAP_ALL_FROZEN))
-           all_frozen_set = true;
-   }
-
    /*
     * We're about to do the actual insert -- but check for conflict first, to
     * avoid possibly having to roll back work we've just done.
@@ -2138,14 +2109,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    RelationPutHeapTuple(relation, buffer, heaptup,
                         (options & HEAP_INSERT_SPECULATIVE) != 0);
 
-   /*
-    * If the page is all visible, need to clear that, unless we're only going
-    * to add further frozen rows to it.
-    *
-    * If we're only adding already frozen rows to a page that was empty or
-    * marked as all visible, mark it as all-visible.
-    */
-   if (PageIsAllVisible(BufferGetPage(buffer)) && !(options & HEAP_INSERT_FROZEN))
+   if (PageIsAllVisible(BufferGetPage(buffer)))
    {
        all_visible_cleared = true;
        PageClearAllVisible(BufferGetPage(buffer));
@@ -2153,13 +2117,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
                            ItemPointerGetBlockNumber(&(heaptup->t_self)),
                            vmbuffer, VISIBILITYMAP_VALID_BITS);
    }
-   else if (all_frozen_set)
-   {
-       /* We only ever set all_frozen_set after reading the page. */
-       Assert(page);
-
-       PageSetAllVisible(page);
-   }
 
    /*
     * XXX Should we set PageSetPrunable on this page ?
@@ -2207,8 +2164,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
        xlrec.flags = 0;
        if (all_visible_cleared)
            xlrec.flags |= XLH_INSERT_ALL_VISIBLE_CLEARED;
-       if (all_frozen_set)
-           xlrec.flags = XLH_INSERT_ALL_FROZEN_SET;
        if (options & HEAP_INSERT_SPECULATIVE)
            xlrec.flags |= XLH_INSERT_IS_SPECULATIVE;
        Assert(ItemPointerGetBlockNumber(&heaptup->t_self) == BufferGetBlockNumber(buffer));
@@ -2257,29 +2212,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 
    END_CRIT_SECTION();
 
-   /*
-    * If we've frozen everything on the page, update the visibilitymap. We're
-    * already holding pin on the vmbuffer.
-    *
-    * No need to update the visibilitymap if it had all_frozen bit set before
-    * this insertion.
-    */
-   if (all_frozen_set && ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0))
-   {
-       Assert(PageIsAllVisible(page));
-       Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer));
-
-       /*
-        * It's fine to use InvalidTransactionId here - this is only used when
-        * HEAP_INSERT_FROZEN is specified, which intentionally violates
-        * visibility rules.
-        */
-       visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
-                         InvalidXLogRecPtr, vmbuffer,
-                         InvalidTransactionId,
-                         VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
-   }
-
    UnlockReleaseBuffer(buffer);
    if (vmbuffer != InvalidBuffer)
        ReleaseBuffer(vmbuffer);
@@ -8946,10 +8878,6 @@ heap_xlog_insert(XLogReaderState *record)
    ItemPointerSetBlockNumber(&target_tid, blkno);
    ItemPointerSetOffsetNumber(&target_tid, xlrec->offnum);
 
-   /* check that the mutually exclusive flags are not both set */
-   Assert(!((xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED) &&
-            (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)));
-
    /*
     * The visibility map may need to be fixed even if the heap page is
     * already up-to-date.

diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index d34edb4190c8b328303b8305ac881d680a82ce54..c47c7522ca64e89323c1538c18ff9cf87dc3150f 100644 (file)
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -407,19 +407,19 @@ RelationGetBufferForTuple(Relation relation, Size len,
         * target.
         */
        targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
-   }
 
-   /*
-    * If the FSM knows nothing of the rel, try the last page before we give
-    * up and extend.  This avoids one-tuple-per-page syndrome during
-    * bootstrapping or in a recently-started system.
-    */
-   if (targetBlock == InvalidBlockNumber)
-   {
-       BlockNumber nblocks = RelationGetNumberOfBlocks(relation);
+       /*
+        * If the FSM knows nothing of the rel, try the last page before we
+        * give up and extend.  This avoids one-tuple-per-page syndrome during
+        * bootstrapping or in a recently-started system.
+        */
+       if (targetBlock == InvalidBlockNumber)
+       {
+           BlockNumber nblocks = RelationGetNumberOfBlocks(relation);
 
-       if (nblocks > 0)
-           targetBlock = nblocks - 1;
+           if (nblocks > 0)
+               targetBlock = nblocks - 1;
+       }
    }
 
 loop: