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: http://www.postgresql.org/mailpref/pgsql-hackers
