Fix bug due to confusion about what IsMVCCSnapshot means In 0b96e734c59 I (Andres) relied on page_collect_tuples() being called only with an MVCC snapshot, and added assertions to that end, but did not realize that IsMVCCSnapshot() allows both proper MVCC snapshots and historical snapshots, which behave quite similarly to MVCC snapshots.
Unfortunately that can lead to incorrect visibility results during logical decoding, as a historical snapshot is interpreted as a plain MVCC snapshot. The only reason this wasn't noticed earlier is that it's hard to reach as most of the time there are no sequential scans during logical decoding. To fix the bug and avoid issues like this in the future, split IsMVCCSnapshot() into IsMVCCSnapshot() and IsMVCCLikeSnapshot(), where now only the latter includes historic snapshots. One effect of this is that during logical decoding no page-at-a-time snapshots are used, as otherwise runtime branches to handle historic snapshots would be needed in some performance critical paths. Given how uncommon sequential scans are during logical decoding, that seems acceptable. Author: Antonin Houska <[email protected]> Reported-by: Antonin Houska <[email protected]> Discussion: https://postgr.es/m/61812.1770637345@localhost Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ce5d489166e5c8437aa1a35f462f67f1aeae87d4 Modified Files -------------- src/backend/access/heap/heapam_handler.c | 2 +- src/backend/access/index/indexam.c | 2 +- src/backend/access/nbtree/nbtree.c | 2 +- src/include/utils/snapmgr.h | 21 ++++++++++++++++++--- 4 files changed, 21 insertions(+), 6 deletions(-)
