Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

Etsuro Fujita wrote: > Done. Attached is an updated version of the patch. Pushed, thanks. I kinda wonder why this struct member has a name that doesn't match the naming convention in the rest of the struct, and also why isn't it documented in the comment above the struct definition. But

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/02/03 3:31, Alvaro Herrera wrote: Etsuro Fujita wrote: Done. Attached is an updated version of the patch. Pushed, thanks. Thank you! I kinda wonder why this struct member has a name that doesn't match the naming convention in the rest of the struct, and also why isn't it

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

Etsuro Fujita wrote: > On 2016/01/28 12:13, Robert Haas wrote: > >I don't think this is a good idea. Most of the time, no system > >columns will be present, and we'll just be scanning the Bitmapset > >twice rather than once. Sure, that doesn't take many extra cycles, > >but if the point of all

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/28 18:15, Alvaro Herrera wrote: Etsuro Fujita wrote: On 2016/01/28 12:13, Robert Haas wrote: I don't think this is a good idea. Most of the time, no system columns will be present, and we'll just be scanning the Bitmapset twice rather than once. Sure, that doesn't take many

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On Thu, Jan 21, 2016 at 5:55 AM, Etsuro Fujita wrote: > On 2016/01/21 7:04, Alvaro Herrera wrote: >> Etsuro Fujita wrote: >>> >>> On second thought, I noticed that detecting whether we see a system >>> column >>> that way needs more cycles in cases where the

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/28 12:13, Robert Haas wrote: On Thu, Jan 21, 2016 at 5:55 AM, Etsuro Fujita wrote: On 2016/01/21 7:04, Alvaro Herrera wrote: Etsuro Fujita wrote: On second thought, I noticed that detecting whether we see a system column that way needs more cycles in

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/21 7:04, Alvaro Herrera wrote: Etsuro Fujita wrote: On second thought, I noticed that detecting whether we see a system column that way needs more cycles in cases where the reltargetlist and the restriction clauses don't contain any system columns. ISTM that such cases are rather

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

Etsuro Fujita wrote: > On second thought, I noticed that detecting whether we see a system column > that way needs more cycles in cases where the reltargetlist and the > restriction clauses don't contain any system columns. ISTM that such cases > are rather common, so I'm inclined to keep that

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/15 19:00, Etsuro Fujita wrote: On 2016/01/12 18:00, Etsuro Fujita wrote: On 2016/01/12 2:36, Alvaro Herrera wrote: I wonder, --- 2166,2213 } /* ! * If rel is a base relation, detect whether any system columns are ! * requested from the rel. (If rel

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/12 18:00, Etsuro Fujita wrote: On 2016/01/12 2:36, Alvaro Herrera wrote: I wonder, --- 2166,2213 } /* ! * If rel is a base relation, detect whether any system columns are ! * requested from the rel. (If rel is a join relation, rel->relid will be !

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/12 2:36, Alvaro Herrera wrote: I wonder, --- 2166,2213 } /* !* If rel is a base relation, detect whether any system columns are !* requested from the rel. (If rel is a join relation, rel->relid will be !* 0, but there can be no Var in

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

I wonder, > --- 2166,2213 > } > > /* > ! * If rel is a base relation, detect whether any system columns are > ! * requested from the rel. (If rel is a join relation, rel->relid will > be > ! * 0, but there can be no Var in the target list with relid 0, so we

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2015/12/23 2:47, Robert Haas wrote: On Tue, Dec 22, 2015 at 7:32 AM, Michael Paquier wrote: Moved to next CF because of a lack of reviews. Thanks, Michael! I just took a look at this. I think the basic idea of this patch is good, but the comments need some

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On Wed, Sep 9, 2015 at 7:08 PM, Etsuro Fujita wrote: > On 2015/07/10 18:40, Etsuro Fujita wrote: >> To save cycles, I modified create_foreignscan_plan so that it detects >> whether any system columns are requested if scanning a base relation. >> Also, I revised other

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On Tue, Dec 22, 2015 at 7:32 AM, Michael Paquier wrote: > On Wed, Sep 9, 2015 at 7:08 PM, Etsuro Fujita > wrote: >> On 2015/07/10 18:40, Etsuro Fujita wrote: >>> To save cycles, I modified create_foreignscan_plan so that it detects >>>

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2015/07/10 18:40, Etsuro Fujita wrote: > To save cycles, I modified create_foreignscan_plan so that it detects > whether any system columns are requested if scanning a base relation. > Also, I revised other code there a little bit. Attached is an updated version of the patch. The previous

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2015/08/30 13:06, David Rowley wrote: It's likely not worth changing if there's cases when it'll be slower, but curiosity got the better of me and I wondered how extreme a case it would take to actually see a slowdown, and per my benchmark results the first used column would have to be about

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 28 August 2015 at 22:20, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: On 2015/07/22 15:25, Etsuro Fujita wrote: On 2015/07/10 21:59, David Rowley wrote: I just glanced at this and noticed that the method for determining if there's any system columns could be made a bit nicer. /*

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2015/07/22 15:25, Etsuro Fujita wrote: On 2015/07/10 21:59, David Rowley wrote: On 10 July 2015 at 21:40, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: To save cycles, I modified create_foreignscan_plan so that it detects whether any system

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2015/07/10 21:59, David Rowley wrote: On 10 July 2015 at 21:40, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: To save cycles, I modified create_foreignscan_plan so that it detects whether any system columns are requested if scanning a base

[HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

To save cycles, I modified create_foreignscan_plan so that it detects whether any system columns are requested if scanning a base relation. Also, I revised other code there a little bit. For ExecInitForeignScan, I simplified the code there to determine the scan tuple type, whith seems to me

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 10 July 2015 at 21:40, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: To save cycles, I modified create_foreignscan_plan so that it detects whether any system columns are requested if scanning a base relation. Also, I revised other code there a little bit. For ExecInitForeignScan, I