Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan - Mailing list pgsql-hackers

From David Rowley
Subject Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan
Date
Msg-id CAKJS1f8EOCn+3GhzdVbRD74s=Jf9JpFGxgW6D_P1BuVGb7vaXw@mail.gmail.com
Whole thread Raw
In response to Minor code improvements to create_foreignscan_plan/ExecInitForeignScan  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Responses Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan
List pgsql-hackers
On 10 July 2015 at 21:40, Etsuro Fujita <fujita.etsuro@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 simplified the code there to determine the
scan tuple type, whith seems to me complex beyound necessity.  Maybe
that might be nitpicking, though.


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.

Regards

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

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Fillfactor for GIN indexes
Next
From: Beena Emerson
Date:
Subject: Re: Support for N synchronous standby servers - take 2