On 2015/07/10 21:59, David Rowley wrote:
On 10 July 2015 at 21:40, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp
<mailto: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.
For the latter, I misunderstood that the latest foreign-join pushdown
patch allows fdw_scan_tlist to be set to a targetlist even for simple
foreign table scans. Sorry for that, but I have a concern about that.
I'd like to mention it in a new thread later.
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.
Good catch!
Will update the patch (and drop the ExecInitForeignScan part from the
patch).
Sorry for the delay.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
