On Mon, Apr 18, 2022 at 10:53 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: > > I still think it would be better to have GetOldestSnapshot() be > > smarter and refuse to return the catalog snapshot. For one thing, that > > way we'd be testing for the problem case in non-assert builds also. > > I was wondering about that too. On the other hand, given that > we know this area is squishy, transforming fails-in-assert-builds > to fails-everywhere is not necessarily desirable.
I agree that it's a little unclear. In general, I think if we're going to blow up and die, doing it closer to the place where the problem is happening is for the best. On the other hand, if in most practical cases we're going to stumble through and get the right answer anyway, then it's maybe not great to break a bunch of accidentally-working cases. However, it does strikes me that this principal could easily be overdone. init_toast_snapshot() could pick a random snapshot (or take a new one) in order to call InitToastSnapshot() and that would often work fine. Yet, upon realizing that things are busted, it chooses to error out instead. I approve of that choice, and don't think we should rule out the idea of making that check more robust. -- Robert Haas EDB: http://www.enterprisedb.com
