From: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan |
Date: | 2015-08-28 10:20:41 |
Message-ID: | 55E035F9.1030008@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp
>> <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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.
>> 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.
Anyway, I'll add this to the upcoming CF.
Best regards,
Etsuro Fujita
From | Date | Subject | |
---|---|---|---|
Next Message | jacques klein | 2015-08-28 10:30:54 | NOTIFY in Background Worker |
Previous Message | Fabien COELHO | 2015-08-28 09:17:31 | Re: patch: version_stamp.pl: Add Git commit info to version if 'git' is specified |