Use PageIndexTupleOverwrite() within nbtsort.c

Started by Peter Geogheganalmost 7 years ago3 messageshackers
Jump to latest

Attached patch slightly simplifies nbtsort.c by making it use
PageIndexTupleOverwrite() to overwrite the last right non-pivot tuple
with the new high key (pivot tuple). PageIndexTupleOverwrite() is
designed so that code like this doesn't need to delete and re-insert
to replace an existing tuple.

This slightly simplifies the code, and also makes it marginally
faster. I'll add this to the 2019-09 CF.

--
Peter Geoghegan

Attachments:

0001-Overwrite-lastright-item-with-highkey-in-nbtsort.c.patchapplication/octet-stream; name=0001-Overwrite-lastright-item-with-highkey-in-nbtsort.c.patchDownload+6-16
#2Anastasia Lubennikova
a.lubennikova@postgrespro.ru
In reply to: Peter Geoghegan (#1)
Re: Use PageIndexTupleOverwrite() within nbtsort.c

16.07.2019 1:12, Peter Geoghegan wrote:

Attached patch slightly simplifies nbtsort.c by making it use
PageIndexTupleOverwrite() to overwrite the last right non-pivot tuple
with the new high key (pivot tuple). PageIndexTupleOverwrite() is
designed so that code like this doesn't need to delete and re-insert
to replace an existing tuple.

This slightly simplifies the code, and also makes it marginally
faster. I'll add this to the 2019-09 CF.

I'm okay with this patch.

Should we also update similar code in _bt_mark_page_halfdead()?
I attached a new version of the patch with this change.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

0001-Overwrite-lastright-item-with-highkey_v2.patchtext/x-patch; name=0001-Overwrite-lastright-item-with-highkey_v2.patchDownload+9-19
In reply to: Anastasia Lubennikova (#2)
Re: Use PageIndexTupleOverwrite() within nbtsort.c

On Tue, Aug 6, 2019 at 8:30 AM Anastasia Lubennikova
<a.lubennikova@postgrespro.ru> wrote:

Should we also update similar code in _bt_mark_page_halfdead()?
I attached a new version of the patch with this change.

Pushed.

At first I thought that there might be a problem with doing the same
thing within _bt_mark_page_halfdead(), because we still won't use
PageIndexTupleOverwrite() in the corresponding recovery routine -- in
theory, that could break WAL consistency checking because the redo
routine works by reconstructing a half-deleted leaf page from scratch,
resulting in a logically equivalent though physically different page
(even after masking within btree_mask()). However, I eventually
decided that you had it right. Your _bt_mark_page_halfdead() change is
clearer overall and doesn't break WAL consistency checking in
practice, for reasons that are no less obvious than before.

Thanks!
--
Peter Geoghegan