On 2016/04/08 22:21, Rushabh Lathia wrote:
> On Fri, Apr 8, 2016 at 6:28 PM, Robert Haas <robertmhaas@gmail.com
> <mailto:robertmhaas@gmail.com>> wrote:
> The comment just before the second hunk in the patch says:
>
> * We don't use a PG_TRY block here, so be careful not to
> throw error
> * without releasing the PGresult.
>
> But the patch adds a whole bunch of new things there that seem like
> they can error out, like CHECK_FOR_INTERRUPTS(), for example. Isn't
> that a problem?
> Basically we fetching the PGresult, after the newly added hunk, so there
> should not be any problem.
>
> But yes comment is definitely at wrong place.
>
> PFA patch with correction.
I agree with Rushabh. Thanks for updating the patch!
Another thing I'd like to propose to revise the patch is to call
pgfdw_report_error in the newly added hunk, with the clear argument set
to *false*. The PGresult argument is NULL there, so no need to release
the PGresult.
Attached is an updated patch.
Best regards,
Etsuro Fujita