More vacuum.c refactoring - Mailing list pgsql-hackers

Near the end of repair_frag() in vacuum.c -- under the comment /* clean
moved tuples from last page in Nvacpagelist list */ -- there is code
that marks itemids as unused.  Itemids affected are those referring to
tuples that have been moved off the last page.

This code is very similar to vacuum_page().  The major difference is
that vacuum_page() uses vacpage->offsets while the code in repair_frag()
looks for MOVED_OFF bits in tuple headers.  AFAICS the tuples with the
MOVED_OFF bit set are exactly those referenced by vacpage->offsets.

The attached patch passes make check and make installcheck.  Please
apply unless I'm missing something.

Servus
 Manfred
diff -Ncr ../base/src/backend/commands/vacuum.c src/backend/commands/vacuum.c
*** ../base/src/backend/commands/vacuum.c    Wed Jun  2 21:46:59 2004
--- src/backend/commands/vacuum.c    Thu Jun 10 18:50:26 2004
***************
*** 2288,2355 ****
              vacpage->offsets_free > 0)
          {
              Buffer            buf;
-             Page            page;
-             OffsetNumber    unused[BLCKSZ / sizeof(OffsetNumber)];
-             OffsetNumber    offnum,
-                             maxoff;
-             int                uncnt;
-             int                num_tuples = 0;

              buf = ReadBuffer(onerel, vacpage->blkno);
              LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
!             page = BufferGetPage(buf);
!             maxoff = PageGetMaxOffsetNumber(page);
!             for (offnum = FirstOffsetNumber;
!                  offnum <= maxoff;
!                  offnum = OffsetNumberNext(offnum))
!             {
!                 ItemId    itemid = PageGetItemId(page, offnum);
!                 HeapTupleHeader htup;
!
!                 if (!ItemIdIsUsed(itemid))
!                     continue;
!                 htup = (HeapTupleHeader) PageGetItem(page, itemid);
!                 if (htup->t_infomask & HEAP_XMIN_COMMITTED)
!                     continue;
!
!                 /*
!                 ** See comments in the walk-along-page loop above, why we
!                 ** have Asserts here instead of if (...) elog(ERROR).
!                 */
!                 Assert(!(htup->t_infomask & HEAP_MOVED_IN));
!                 Assert(htup->t_infomask & HEAP_MOVED_OFF);
!                 Assert(HeapTupleHeaderGetXvac(htup) == myXID);
!
!                 itemid->lp_flags &= ~LP_USED;
!                 num_tuples++;
!
!             }
!             Assert(vacpage->offsets_free == num_tuples);
!
!             START_CRIT_SECTION();
!
!             uncnt = PageRepairFragmentation(page, unused);
!
!             /* XLOG stuff */
!             if (!onerel->rd_istemp)
!             {
!                 XLogRecPtr    recptr;
!
!                 recptr = log_heap_clean(onerel, buf, unused, uncnt);
!                 PageSetLSN(page, recptr);
!                 PageSetSUI(page, ThisStartUpID);
!             }
!             else
!             {
!                 /*
!                  * No XLOG record, but still need to flag that XID exists
!                  * on disk
!                  */
!                 MyXactMadeTempRelUpdate = true;
!             }
!
!             END_CRIT_SECTION();
!
              LockBuffer(buf, BUFFER_LOCK_UNLOCK);
              WriteBuffer(buf);
          }
--- 2288,2297 ----
              vacpage->offsets_free > 0)
          {
              Buffer            buf;

              buf = ReadBuffer(onerel, vacpage->blkno);
              LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
!             vacuum_page(onerel, buf, vacpage);
              LockBuffer(buf, BUFFER_LOCK_UNLOCK);
              WriteBuffer(buf);
          }