Tom Lane wrote:
Heikki Linnakangas <[EMAIL PROTECTED]> writes:
BTW: Should we do the synchronization in the non-page-at-a-time mode? It's not many lines of code to do so, but IIRC that codepath is only used for catalog access. System tables really shouldn't grow that big, and if they do we shouldn't be doing seq scans for them anyway.

It occurs to me that there's an actual bug here for catalog access.
The code assumes that it can measure rs_nblocks only once and not worry
about tuples added beyond that endpoint.  But this is only true when
using an MVCC-safe snapshot.  In SnapshotNow mode, in particular, this
could easily lead to missing a valid tuple.  I'm not sure such a bug
has ever been seen in the wild, because we don't do that many heapscans
on system catalogs (and most of the ones we do do have some amount of
higher-level locking preventing a concurrent update).  But we ought to
fix it while we're touching the code now.  That will take re-measuring
rs_nblocks each time we consider stopping, and I think we definitely
will NOT want the syncscan code getting involved.

You would only miss tuples inserted after you began the seqscan. After you've began the scan, you're going to miss any tuples that are inserted before the current position anyway, so stopping the scan early shouldn't do any real harm. It would only be a problem if you do something like:
while(heap_getnext) ...

And expect to see all tuples inserted before acquiring the lock.

Good catch, though. Fixing it is probably a good idea even if it's not a problem for any existing code.

  Heikki Linnakangas

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?


Reply via email to