Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

Thanks Jeevan for your review and comments. PFA the patch which fixes those.

On Tue, Feb 9, 2016 at 5:00 PM, Jeevan Chalke <
jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:

> Hi,
>
> I have reviewed the patch and it looks good to me.
> make/make install/make check is fine (when done without -Wall -Werror).
>
> Here are few comments:
> 1.
> With -Wall -Werror, I see couple of warnings:
>
> postgres_fdw.c: In function ‘estimate_path_cost_size’:
> postgres_fdw.c:2248:13: error: ‘run_cost’ may be used uninitialized in
> this function [-Werror=uninitialized]
>

Done. run_cost was declared in a block enclosing the one where it was used.
So moved run_cost and initialized it. The initialized value is never used.

> postgres_fdw.c: In function ‘conversion_error_callback’:
> postgres_fdw.c:3832:6: error: ‘attname’ may be used uninitialized in this
> function [-Werror=uninitialized]
> cc1: all warnings being treated as errors
> make: *** [postgres_fdw.o] Error 1
>
>
Thanks for catching it. Fixed as well.

> 2. Typo:
> scna_clauses => scan_clauses
>
>
Done.

> 3. Does this new addition requires documentation?
>
>
The patch pg_fdw_doc.patch adds a paragraph about join pushdown in
postgres_fdw documentation.

>
> I did not see any issues with my testing. Code changes are good too.
> Patch has very good test-cases testing everything required. Nice work.
>

Thanks.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company