On Wed, Jan 11, 2017 at 5:45 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
> On 2017/01/05 21:38, Ashutosh Bapat wrote:
>> On Thu, Jan 5, 2017 at 5:51 PM, Etsuro Fujita
>> <fujita.ets...@lab.ntt.co.jp> wrote:
>>> On 2017/01/05 21:11, Ashutosh Bapat wrote:
>>>>> On 2017/01/03 17:28, Ashutosh Bapat wrote:
>>>>>> Also, in this function, if fpinfo->tlist is already set, why do we
>>>>>> want
>>>>>> to
>>>>>> build it again?
>>>> IIUC, for a relation with use_remote_estimates we will deparse the
>>>> query twice and will build the targetlist twice.
>>> That's right.  We could avoid the duplicate work the way you proposed,
>>> but I
>>> was thinking to leave that for another patch.  Should we do that in this
>>> patch?
>> If you are agree that the change is needed, it's better to do it in
>> this patch itself if we can, instead of a one liner patch.
> While working on this, I noticed a weird case.  Consider:
> postgres=# explain verbose select 1 from ft1 left join ft2 on (ft1.a =
> ft2.a) inner join test on (true);
>                                            QUERY PLAN
> -------------------------------------------------------------------------------------------------
>  Nested Loop  (cost=100.00..103.06 rows=1 width=4)
>    Output: 1
>    ->  Foreign Scan  (cost=100.00..102.04 rows=1 width=0)
>          Relations: (public.ft1) LEFT JOIN (public.ft2)
>          Remote SQL: SELECT NULL FROM (public.t1 r1 LEFT JOIN public.t2 r2
> ON (((r1.a = r2.a))))
>    ->  Seq Scan on public.test  (cost=0.00..1.01 rows=1 width=0)
>          Output: test.a, test.b
> (7 rows)
> In this case the fpinfo->tlist of the foreign join is NIL, so whether or not
> the tlist is already built cannot be discriminated by the fpinfo->tlist.  We
> might need another flag to show that the tlist has been built already.
> Since this is an existing issue and we would need to give careful thought to
> this, so I'd like to leave this for another patch.

I think in that case, relation's targetlist will also be NIL or
contain no Var node. It wouldn't be expensive to build it again and
again. That's better than maintaining a new flag. This is just a
suggestion, for an additional check, you might want to check
rel->reltarget->exprs for NIL. But I think we don't need it.

Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to