On Wed, 3 Apr 2024 at 21:49, Robert Haas <robertmhaas@gmail.com> wrote:
> It seems to me that 0001 should either remove the pg_unreachable()
> call or change the break to a return, but not both.
Updated the patch to only remove the pg_unreachable call (and keep the breaks).
> but there's not much value in omitting
> pg_unreachable() from unreachable places, either, so I'm not
> convinced.
But I don't agree with this. Having pg_unreachable in places where it
brings no perf benefit has two main downsides:
1. It's extra lines of code
2. If a programmer/reviewer is not careful about maintaining this
unreachable invariant (now or in the future), undefined behavior can
be introduced quite easily.
Also, I'd expect any optimizing compiler to know that the code after
the loop is already unreachable if there are no break/goto statements
in its body.
> I agree with Tristan's analysis of 0002.
Updated 0002, to only change the comment. But honestly I don't think
using "default" really introduces many chances for future bugs in this
case, since it seems very unlikely we'll ever add new variants to the
PostgresPollingStatusType enum.