_bt_delitems_delete() should use XLogRegisterBufData(), not XLogRegisterData()

Started by Peter Geogheganover 6 years ago2 messageshackers
Jump to latest

The RelationNeedsWAL() code block within _bt_delitems_delete() has had
the following comment for many years now:

/*
* We need the target-offsets array whether or not we store the whole
* buffer, to allow us to find the latestRemovedXid on a standby
* server.
*/
XLogRegisterData((char *) itemnos, nitems * sizeof(OffsetNumber));

However, we don't actually need to do it that way these days. We won't
go on to determine a latestRemovedXid on a standby as of commit
558a9165e08 (that happens on the primary instead), so the comment
seems wrong.

Rather than just changing the comment, I propose that we tweak the
behavior of _bt_delitems_delete() to match its sibling function
_bt_delitems_vacuum(). That is, it should use XLogRegisterBufData(),
not XLogRegisterData(). This is cleaner, and ought to be a minor win.

Attached patch shows what I have in mind. The new comment block has
been copied from _bt_delitems_vacuum().

--
Peter Geoghegan

Attachments:

0001-Associate-LP_DEAD-offsets-with-WAL-record-s-buffer.patchapplication/x-patch; name=0001-Associate-LP_DEAD-offsets-with-WAL-record-s-buffer.patchDownload+5-5
In reply to: Peter Geoghegan (#1)
Re: _bt_delitems_delete() should use XLogRegisterBufData(), not XLogRegisterData()

On Wed, Jan 1, 2020 at 1:00 PM Peter Geoghegan <pg@bowt.ie> wrote:

Attached patch shows what I have in mind. The new comment block has
been copied from _bt_delitems_vacuum().

I also think that the WAL record and function signature of
_bt_delitems_delete() should be brought closer to
_bt_delitems_vacuum(). Attached patch does it that way.

I intend to commit this in the next day or two, barring any objections.

--
Peter Geoghegan

Attachments:

v2-0001-Associate-LP_DEAD-offsets-with-WAL-record-s-buffe.patchapplication/octet-stream; name=v2-0001-Associate-LP_DEAD-offsets-with-WAL-record-s-buffe.patchDownload+24-23