Re: [BUGS] BUG #4599: bugfix for contrib/dblink module - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [BUGS] BUG #4599: bugfix for contrib/dblink module
Date
Msg-id 28719.1230996378@sss.pgh.pa.us
Whole thread Raw
In response to Re: [BUGS] BUG #4599: bugfix for contrib/dblink module  (Joe Conway <mail@joeconway.com>)
Responses Re: [BUGS] BUG #4599: bugfix for contrib/dblink module  (Joe Conway <mail@joeconway.com>)
List pgsql-hackers
Joe Conway <mail@joeconway.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


pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: reloptions and toast tables
Next
From: Simon Riggs
Date:
Subject: Re: generic reloptions improvement