Re: [BUGS] BUG #4599: bugfix for contrib/dblink module

Joe Conway <mail(at)joeconway(dot)com> writes:
> On line 795 below, fail should get set to PG_GETARG_BOOL(1). However, as
> line 842 is about to be executed, fail is still set to true, even though
> PG_GETARG_BOOL(1) is clearly false. Any ideas?

I can't duplicate that here, but my first reaction on studying this code
is "ick!". Having a non-set-returning function calling the SRF
infrastructure (and not bothering to clean it up on exit, either) is
just horrid --- I have no idea what side-effects that might have, but at
the very least there's going to be a memory leak. Trying to implement
three significantly different functions as one function with a maze of
if's is not good style in any case.

I think you should break those three functions apart. There is no value
in having send_query share any code with the others. It might be
feasible to have the other two share a subroutine that collects the
result data.

regards, tom lane