Re: ThisTimeLineID can be used uninitialized

On Tue, Oct 19, 2021 at 7:30 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> Hah. Yeah, when you can do things like that and the tests don't break,
> that indicates a problem in the tests.

I *think* the problem is actually in the code, not the tests. In other
words, from what I can tell, we copy the bogus timeline value (0, or a
random number) into several places, but then eventually overwrite all
copies of that value with a correct value before using it for
anything. So in other words I think that the comment saying that this
code is initializing values that WalSndSegmentOpen is going to need is
just wrong. I don't completely understand why it's wrong, but I think
it IS wrong.

> > Istm we should introduce an InvalidTimeLineID, and explicitly initialize
> > sendTimeLine to that, and assert that it's valid / invalid in a bunch of
> > places?
>
> That's not a bad idea; it'll help discover bogus code. Obviously, some
> additional tests wouldn't harm -- we have a lot more coverage now than
> in embarrasingly recent past, but it can still be improved.

+1.

--
Robert Haas
EDB: http://www.enterprisedb.com