Re: Possibly too stringent Assert() in b-tree code

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Sep 19, 2016 at 7:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> I think you have a valid point.  It seems we don't need to write WAL
>> for reuse page (aka don't call _bt_log_reuse_page()), if the page is
>> new, as the only purpose of that log is to handle conflict based on
>> transaction id stored in special area which will be anyway zero.

> +1.

This is clearly an oversight in Simon's patch fafa374f2, which introduced
this code without any consideration for the possibility that the page
doesn't have a valid special area.  We could prevent the crash by
doing nothing if PageIsNew, a la
               if (_bt_page_recyclable(page))               {                   /*                    * If we are
generatingWAL for Hot Standby then create a                    * WAL record that will allow us to conflict with queries
                  * running on standby.                    */
 
-                   if (XLogStandbyInfoActive() && RelationNeedsWAL(rel))
+                   if (XLogStandbyInfoActive() && RelationNeedsWAL(rel) &&
+                       !PageIsNew(page))                   {                       BTPageOpaque opaque =
(BTPageOpaque)PageGetSpecialPointer(page);
 
                       _bt_log_reuse_page(rel, blkno, opaque->btpo.xact);                   }
                   /* Okay to use page.  Re-initialize and return it */

but I'm not very clear on whether this is a safe fix, mainly because
I don't understand what the reuse WAL record really accomplishes.
Maybe we need to instead generate a reuse record with some special
transaction ID indicating worst-case assumptions?
        regards, tom lane