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 CAFjFpRc_Dm7jQpcND+nG_Kek=Xpagn8Kcm_SjnqYC+HwmYkSZw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Дерево обсуждения
On Fri, Jan 29, 2016 at 9:51 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Jan 28, 2016 at 11:26 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> 2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below

This patch no longer quite applies because of conflicts with one of
your other patches that I applied today (cf. commit
fbe5a3fb73102c2cfec11aaaa4a67943f4474383).

And then I broke it some more by committing a patch to extract
deparseLockingClause from postgresGetForeignPlan and move it to
deparse.c, but that should pretty directly reduce the size of this
patch.  I wonder if there are any other bits of refactoring of that
sort that we can do in advance of landing the main patch, just to
simplify review.  But I'm not sure there are: this patch removes very
little existing code; it just adds a ton of new stuff.

PFA patch to move code to deparse SELECT statement into a function deparseSelectStmtForRel(). This code is duplicated in estimate_path_cost_size() and postgresGetForeignPlan(), so moving it into a function avoids that duplication. As a side note, estimate_path_cost_size() doesn't add FOR SHARE/UPDATE clause to the statement being EXPLAINed, even if the actual statement during execution would have it. This difference looks unintentional to me. This patch corrects it as well. appendOrderByClause and appendWhereClause both create a context within themselves and pass it to deparseExpr. This patch creates the context once in deparseSelectStmtForRel() and then passes it to the other deparse functions avoiding repeated context creation.
 
copy_path_for_epq_recheck() and friends are really ugly.  Should we
consider just adding copyObject() support for those node types
instead?

the note in copyfuncs.c says
 * We also do not support copying Path trees, mainly
 * because the circular linkages between RelOptInfo and Path nodes can't
 * be handled easily in a simple depth-first traversal.

We may avoid that by just copying the pointer to RelOptInfo and not copy the whole RelOptInfo. The other problem is paths in epq_paths will be copied as many times as the number of 2-way joins pushed down. Let me give it a try and produce patch with that.

I'm sure there's more -- this is a huge patch and I don't fully
understand it yet -- but I'm out of energy for tonight.


Thanks a lot for your comments and moving this patch forward.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Вложения

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

Предыдущее
От: Etsuro Fujita
Дата:
Сообщение: Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)
Следующее
От: Ashutosh Bapat
Дата:
Сообщение: Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)