Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: cf/5816~1
Choose a base ref
...
head repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: cf/5816
Choose a head ref
  • 2 commits
  • 3 files changed
  • 2 contributors

Commits on Aug 6, 2025

  1. Stop conditioning nbtree actions on pin being held. 

    Commit 2ed5b87, which taught nbtree to avoid holding onto a leaf page
buffer pin across the btgettuple calls for certain index scans, made it
possible for a scan position (either so->currPos or so->markPos) to be a
valid position without having a buffer pin held/a valid value in 'buf'.
The same commit introduced an idiom around buffer pin management: at the
point when a scan position was invalidated, a call to the function-like
macro BTScanPosUnpinIfPinned() released the buffer pin iff the relevant
position's 'buf' field was set to a non-InvalidBuffer value.  Experience
with bugfix commit 7c319f5 suggests that this idiom is rather brittle.

Finish off the work started by recent commit e6eed40: fully move over
to conditioning releasing buffer pins on whether the scan in question is
a so->dropPin scan.  If it is, then _bt_drop_lock_and_maybe_pin must
have already dropped the buffer pin earlier on.  If it's not, then
_bt_drop_lock_and_maybe_pin certainly won't have dropped the pin when
called, so doing so is the responsibility of the pos-invalidating code.
BTScanPosUnpinIfPinned() is no longer needed, so get rid of it.  Keep
BTScanPosIsPinned(), but only call it from assertions going forward.

Now that the rules around buffer pins are a lot clearer, we can remove
all IncrBufferRefCount calls from nbtree.  They aren't truly necessary;
both of the IncrBufferRefCount call sites effectively just incremented
and then immediately decremented the backend-local pin count.  We can
safely avoid changing the buffer refcount by explicitly "transferring"
ownership of the pin from the old obsolescent position to the new one
that replaces it. (Actually, the btrestrpos caller _did_ keep around a
so->markPos/second pin, even after it returned, because the mark had to
be kept around in case it needed to be restored again some time later.
But we can safely keep such a mark around by setting so->markItemIndex,
and then invalidating the mark's original so->markPos representation.
This approach makes btrestrpos symmetrical with the approach taken by
_bt_steppage, which is the other former IncrBufferRefCount caller.)

This hardening/refactoring is a follow-up to bugfix commit 7c319f5.

Author: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAH2-Wzm3a8i31aBXi6oQt9uG7m1-xpX-MXjMMYoDJH=sBj1jnA@mail.gmail.com
    @petergeoghegan
    petergeoghegan authored and Commitfest Bot committed Aug 6, 2025
    Configuration menu
    Copy the full SHA
    172f906 View commit details
    Browse the repository at this point in the history

  2. [CF 5816] v1 - Removing BTScanPosUnpinIfPinned idiom from nbtree, sim… 

    …plifying mark/restore support

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5816

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/CAH2-WzkSj_tPE7UP2-+0D6GO6=2PX55mcCMBwP0MuAePomKfAg@mail.gmail.com
Author(s): Peter Geoghegan
    Commitfest Bot committed Aug 6, 2025
    Configuration menu
    Copy the full SHA
    d51ef03 View commit details
    Browse the repository at this point in the history
Loading