On Mon, Apr 18, 2022 at 10:53 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@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
