Re: Push down more full joins in postgres_fdw

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: Push down more full joins in postgres_fdw
Дата
Msg-id CAFjFpReTQXVTVWbi_HY1nJYNeyeBpE3DzQXST-cj8wRP4jPYCg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Push down more full joins in postgres_fdw  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Ответы Re: Push down more full joins in postgres_fdw
Список pgsql-hackers
Also another point

I guess, this note doesn't add much value in the given context.
Probably we should remove it.
+            * Note: the tlist would have one-to-one correspondence with the
+            * joining relation's reltarget->exprs because (1) the above test
+            * on PHVs guarantees that the reltarget->exprs doesn't contain
+            * any PHVs and (2) the joining relation's local_conds is NIL.
+            * This allows us to search the targetlist entry matching a given
+            * Var node from the tlist in get_subselect_alias_id.

On Fri, Nov 18, 2016 at 5:10 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
>>
>>     /*
>>      * For a join relation or an upper relation, use
>> deparseExplicitTargetList.
>>      * Likewise, for a base relation that is being deparsed as a subquery,
>> in
>>      * which case the caller would have passed tlist that is non-NIL, use
>> that
>>      * function.  Otherwise, use deparseTargetList.
>>      */
>
> This looks correct. I have modified it to make it simple in the given
> patch. But, I think we shouldn't refer to a function e.g.
> deparseExplicitTargetlist() in the comment. Instead we should describe
> the intent e.g. "deparse SELECT clause from the given targetlist" or
> "deparse SELECT clause from attr_needed".
>
>>
>>>> (3) I don't think we need this in isSubqueryExpr, so I removed it from
>>>> the
>>>> patch:
>>>>
>>>> +       /* Keep compiler happy. */
>>>> +       return false;
>>
>>
>>> Doesn't that cause compiler warning, saying "non-void function
>>> returning nothing" or something like that. Actually, the "if
>>> (bms_is_member(node->varno, outerrel->relids))" ends with a "return"
>>> always. Hence we don't need to encapsulate the code in "else" block in
>>> else { }. It could be taken outside.
>>
>>
>> Yeah, I think so too, but I like the "if () { } else { }" coding.  That
>> coding can be found in other places in core, eg,
>> operator_same_subexprs_lookup.
>
> OK.
>
>
>>
>>>> Done.  I modified the patch as proposed; create the tlist by
>>>> build_tlist_to_deparse in foreign_join_ok, if needed, and search the
>>>> tlist
>>>> by tlist_member.  I also added a new member "tlist" to PgFdwRelationInfo
>>>> to
>>>> save the tlist created in foreign_join_ok.
>>
>>
>>> Instead of adding a new member, you might want to reuse grouped_tlist
>>> by renaming it.
>>
>>
>> Done.
>
> Right now, we are calculating tlist whether or not the ForeignPath
> emerges as the cheapest path. Instead we should calculate tlist, the
> first time we need it and then add it to the fpinfo.
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company



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



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

Предыдущее
От: Dilip Kumar
Дата:
Сообщение: Re: Fun fact about autovacuum and orphan temp tables
Следующее
От: Albe Laurenz
Дата:
Сообщение: Re: Parallel execution and prepared statements