On 2021/11/19 22:13, Bharath Rupireddy wrote:
> How about adding the warning message in pgfdw_abort_cleanup instead of
> pgfdw_get_cleanup_result?
>
> Just before this in pgfdw_abort_cleanup seems better to me.
I was thinking pgfdw_get_cleanup_result() is better because it can
easily report different warning messages based on cases of a timeout
or connection failure, respectively. Since pgfdw_get_cleanup_result()
returns true in both those cases, ISTM that it's not easy to
distinguish them in pgfdw_abort_cleanup().
Anyway, attached is the patch (pgfdw_get_cleanup_result_v1.patch)
that makes pgfdw_get_cleanup_result() report a warning message.
> Yeah, this seems to be an opportunity. But, the function should deal
> with the timeout separately, I'm concerned that the function will
> eventually be having if (timeout_param_specified) { } else { } sort
> of code. We can see how much duplicate code we save here vs the
> readability or complexity that comes with the single function.
Please see the attached patch (refactor_pgfdw_get_result_v1.patch).
This is still WIP, but you can check how much the refactoring can
simplify the code.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
