Re: Avoid unused value (src/fe_utils/print.c) - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Avoid unused value (src/fe_utils/print.c)
Date
Msg-id CAEudQArH36OfEnjKpM=KWnmQaRa3XH_rGyu5ai3U_RaA9LpdEQ@mail.gmail.com
Whole thread Raw
In response to Re: Avoid unused value (src/fe_utils/print.c)  (Marko Tiikkaja <marko@joh.to>)
Responses Re: Avoid unused value (src/fe_utils/print.c)
List pgsql-hackers
Em ter., 11 de jul. de 2023 às 19:34, Marko Tiikkaja <marko@joh.to> escreveu:
On Thu, Jul 6, 2023 at 5:37 PM Karina Litskevich
<litskevichkarina@gmail.com> wrote:
> My point is, technically right now you won't see any difference in output
> if you remove the line. Because if we get to that line the need_recordsep
> is already true. However, understanding why it is true is complicated. That's
> why if you remove the line people who read the code will wonder why we don't
> need a separator after "fputs"ing a footer. So keeping that line will make
> the code more readable.
> Moreover, removing the line will possibly complicate the future maintenance.
> As I wrote in the part you just quoted, if the function changes in the way
> that need_recordsep is not true right before printing footers any more, then
> output will be unexpected.

I agree with Karina here.  Either this patch should keep the
"need_recordsep = true;" line, thus removing the no-op assignment to
false and making the code slightly less unreadable; or the entire
function should be refactored for readability.
 As there is consensus to keep the no-op assignment,
I will go ahead and reject the patch.

regards,
Ranier Vilela

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: Reducing connection overhead in pg_upgrade compat check phase
Next
From: Andres Freund
Date:
Subject: Re: Cleaning up threading code