16.07.2019 2:16, Peter Geoghegan wrote:
Attached patch slightly simplifies _bt_getstackbuf() by making it
accept a child BlockNumber argument, rather than requiring that
callers store the child block number in the parent stack item's
bts_btentry field. We can remove the bts_btentry field from the
BTStackData struct, because we know where we ended up when we split a
page and need to relocate parent to insert new downlink -- it's only
truly necessary to remember what pivot tuple/downlink we followed to
arrive at the page being split. There is no point in remembering the
child block number during our initial descent of a B-Tree, since it's
never actually used at a later point, and can go stale immediately
after the buffer lock on parent is released. Besides,
_bt_getstackbuf() callers can even redefine the definition of child to
be child's right sibling after the descent is over. For example, this
happens when we move right, or when we step right during unique index
insertion.
This slightly simplifies the code. Our stack is inherently
approximate, because we might have to move right for a number of
reasons.
I'll add the patch to the 2019-09 CF.
The refactoring is clear, so I set Ready for committer status.
I have just a couple of notes about comments:
1) I think that it's worth to add explanation of the case when we use
right sibling to this comment:
+ * stack to work back up to the parent page. We use the
child block
+ * number (or possibly the block number of a page to its
right)
2) It took me quite some time to understand why does page deletion case
doesn't need a lock.
I propose to add something like "For more see comments for
_bt_lock_branch_parent()" to this line:
Page deletion caller
+ * can get away with a lock on leaf level page when
locating topparent
+ * downlink, though.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
