Re: Better testing coverage and unified coding for plpgsql loops

On Tue, Jan 2, 2018 at 10:08 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
>> Darafei "Komяpa" Praliaskouski wrote:
>>> - can this macro become a function?
>
>> The "exit_action" argument makes it tough. It can probably be done --
>> it seems to require contorting the one callsite that uses "goto" though.
>
> It could be converted into a function returning bool, a la
>
> if (!loop_rc_processing(...))
> break;
>
> but then the burden is on you to show there's negligible performance
> impact, a question that doesn't arise when just macro-izing existing
> code. I suppose the function could be made inline, but then we're
> right back to the question of how well lcov will display the actual
> code coverage.

I prefer writing this sort of thing using a function call and
dispatching on the return value, as you suggest in the text quoted
here. Long macros that involve a zillion continuation lines are hard
to edit, and often hard to step through properly in a debugger. It
may be that in this case it doesn't matter much because, as you said
in the other email, this code may have no bugs and never get modified;
if so, we don't care whether it's hard to edit and debug the code. Of
course, then we also don't really need tests for it, either.

I disagree that Alvaro or Darafei must carry the burden of proving
that using a function rather than a macro doesn't slow anything down.
In general, it's not like performance concerns unilaterally trump
readability. But in this case, I suspect it would be hard to prove
anything at all. It seems unlikely to be in the first place that any
change would be anything more than noise, and there's the sort of
broader issues that duplicating the code makes the binary bigger,
which carries a distributed cost of its own. I'm not sure how you'd
even design a fair test for something like this.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company