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

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)
Дата
Msg-id CAFjFpRfPCim3ooCN_=CbT2V3tAKPJC3_4xpeqtd6Fs3y_kVy3Q@mail.gmail.com
обсуждение исходный текст
Ответ на Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
Ответы Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Дерево обсуждения
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.chalke@enterprisedb.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
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: Support for N synchronous standby servers - take 2
Следующее
От: Amit Kapila
Дата:
Сообщение: Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby