Re: ThisTimeLineID can be used uninitialized

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>, amul sul <sulamul@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-10-20T13:08:57Z
Lists: pgsql-hackers
On Tue, Oct 19, 2021 at 7:30 PM Alvaro Herrera <alvherre@alvh.no-ip.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



Commits

  1. Remove useless code from CreateReplicationSlot.