Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On Thu, Jan 21, 2016 at 5:55 AM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> 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 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 code as-is.
>
>> Ah, I see now what you did there. I was thinking you'd have the
>> foreach(restrictinfo) loop, then once the loop is complete scan the
>> bitmapset; not scan the bitmap set scan inside the other loop.
>
> Ah, I got to the point. I think that is a good idea. The additional cycles
> for the worst case are bounded and negligible. Please find attached an
> updated patch.

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 this is to micro-optimize this code, that
particular change is going in the wrong direction.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company