Hi,
On 2022-02-19 10:00:02 -0800, Andres Freund wrote:
> See backtrace below [1]. The same problem does *not* exist when starting to
> use the same temp schema in a new session (which drops the old contents
> first), which kind of explains why we've not previously noticed this.
>
> But even so, I'm surprised we haven't noticed this before.
Ah, there's a reason for that. In many cases we'll have a catalog snapshot
registered, which is enough for init_toast_snapshot(). But in Miles' example,
the object dropped just prior ends with catalog invalidations.
Proposed bugfix and test attached.
I think it's ok to backpatch the test. There might be a slight change in
output due to 618c16707a6d6e8f5c83ede2092975e4670201ad not being backpatched,
but that's OK I think.
I think it is dangerous that we return a cached catalog snapshot for things
like GetOldestSnapshot() unless they're also registered or active - we can't
rely on catalog snapshots to be present. Indeed, if we didn't, this bug would
have been found before, as some added assertions confirm.
I don't think we can just ignore the catalog snapshot though, it can be
registered in the future, so it actually is the oldest snapshot. But at least
we should assert that there's some snapshot registered/active. In the attached
patch I've added HaveRegisteredOrActiveSnapshot() and used that in
init_toast_snapshot().
Any better ideas?
Greetings,
Andres Freund