Re: Avoid unused value (src/fe_utils/print.c)

From: Marko Tiikkaja <marko(at)joh(dot)to>
To: Karina Litskevich <litskevichkarina(at)gmail(dot)com>
Cc: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Avoid unused value (src/fe_utils/print.c)
Date: 2023-07-11 22:33:51
Message-ID: CAL9smLBmjWjNd0TOJFyAUjoq54mexZiL7C=Ky9Utr8f9euKizQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 6, 2023 at 5:37 PM Karina Litskevich
<litskevichkarina(at)gmail(dot)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.

.m

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-07-11 22:43:14 Re: Reducing connection overhead in pg_upgrade compat check phase
Previous Message Matthias van de Meent 2023-07-11 21:11:02 Re: Parallel CREATE INDEX for BRIN indexes