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
