On Fri, Dec 23, 2022 at 8:08 AM Justin Pryzby <pry...@telsasoft.com> wrote:

> Makes sense now (in your first message, you said that the problem was
> with "sign", and the patch didn't address the actual problem in
> IS_PLUS()).
>
> One can look and find that the unreachable code was introduced at
> 7a3e7b64a.
>
> With your proposed change, the unreachable line is hit by regression
> tests, which is an improvment.  As is the change to pg_dump.c.

But that now reachable line just unsets a flag that we previously found
unset, right?
And if that line was unreachable, then surely the previous flag-clearing
operation is too?

5669      994426 :                 if (IS_MINUS(Np->Num)) // <- also always
false
5670           0 :                     Np->Num->flag &= ~NUM_F_MINUS;
5671             :             }
5672         524 :             else if (Np->sign != '+' && IS_PLUS(Np->Num))
5673           0 :                 Np->Num->flag &= ~NUM_F_PLUS;

https://coverage.postgresql.org/src/backend/utils/adt/formatting.c.gcov.html

I'm inclined to turn the dead unsets into asserts.

--
John Naylor
EDB: http://www.enterprisedb.com

Reply via email to