Use PageIndexTupleOverwrite() within nbtsort.c

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Use PageIndexTupleOverwrite() within nbtsort.c

Peter Geoghegan-4
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

0001-Overwrite-lastright-item-with-highkey-in-nbtsort.c.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Use PageIndexTupleOverwrite() within nbtsort.c

Anastasia Lubennikova
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


0001-Overwrite-lastright-item-with-highkey_v2.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Use PageIndexTupleOverwrite() within nbtsort.c

Peter Geoghegan-4
On Tue, Aug 6, 2019 at 8:30 AM Anastasia Lubennikova
<[hidden email]> 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