Thread: Use PageIndexTupleOverwrite() within nbtsort.c

Use PageIndexTupleOverwrite() within nbtsort.c

From
Peter Geoghegan
Date:
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

Attachment

Re: Use PageIndexTupleOverwrite() within nbtsort.c

From
Anastasia Lubennikova
Date:
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


Attachment

Re: Use PageIndexTupleOverwrite() within nbtsort.c

From
Peter Geoghegan
Date:
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