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 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/ <http://www.2ndquadrant.com/> PostgreSQL Development, 24x7 Support, Training & Services