Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 28 August 2015 at 22:20, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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.
>>>
>>
> /* Now, are any system columns requested from rel? */
>>> scan_plan->fsSystemCol = false;
>>> for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
>>> {
>>> if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
>>> {
>>> scan_plan->fsSystemCol = true;
>>> break;
>>> }
>>> }
>>>
>>
> I think could just be written as:
>>> /* Now, are any system columns requested from rel? */
>>> if (!bms_is_empty(attrs_used) &&
>>> bms_next_member(attrs_used, -1) < -FirstLowInvalidHeapAttributeNumber)
>>> scan_plan->fsSystemCol = true;
>>> else
>>> scan_plan->fsSystemCol = false;
>>>
>>
> I know you didn't change this, but just thought I'd mention it while
>>> there's an opportunity to fix it.
>>>
>>
> On second thought, I noticed that there is a case when that fix doesn't
> work well; bms_next_member wouldn't be efficient when only the rear
> user-columns are requested from a foreign table that has a large number of
> user-columns. So, I'm inclined to leave that as-is.
>
>
You might be right here. I'd failed to think about that.

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 attnum 500.

I used the attached to benchmark. has_system_columns is the current method,
has_system_columns2 has my changes. Lines are prefixed by the position
where the first (and only) attnum appears in the bitmap set.

1 has_system_columns complete in 1.196000
1 has_system_columns2 complete in 0.170000
2 has_system_columns complete in 1.198000
2 has_system_columns2 complete in 0.167000
4 has_system_columns complete in 1.197000
4 has_system_columns2 complete in 0.170000
8 has_system_columns complete in 1.206000
8 has_system_columns2 complete in 0.203000
16 has_system_columns complete in 1.202000
16 has_system_columns2 complete in 0.237000
32 has_system_columns complete in 1.206000
32 has_system_columns2 complete in 0.232000
64 has_system_columns complete in 1.207000
64 has_system_columns2 complete in 0.268000
128 has_system_columns complete in 1.205000
128 has_system_columns2 complete in 0.368000
256 has_system_columns complete in 1.203000
256 has_system_columns2 complete in 0.780000
512 has_system_columns complete in 1.202000
512 has_system_columns2 complete in 1.302000
1024 has_system_columns complete in 1.199000
1024 has_system_columns2 complete in 3.539000

So, for what it's worth, could be 6 times faster for an "average" sized
table, but hey, we're talking nanoseconds anyway...

Regards

David Rowley
--
David Rowley http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Training & Services