Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

On 2016/01/12 2:36, Alvaro Herrera wrote:
> I wonder,
>
>> --- 2166,2213 ----
>> }
>>
>> /*
>> ! * If rel is a base relation, detect whether any system columns are
>> ! * requested from the rel. (If rel is a join relation, rel->relid will be
>> ! * 0, but there can be no Var in the target list with relid 0, so we skip
>> ! * this in that case. Note that any such system columns are assumed to be
>> ! * contained in fdw_scan_tlist, so we never need fsSystemCol to be true in
>> ! * the joinrel case.) This is a bit of a kluge and might go away someday,
>> ! * so we intentionally leave it out of the API presented to FDWs.
>> */
>> ! scan_plan->fsSystemCol = false;
>> ! if (scan_relid > 0)
>> {
>> ! Bitmapset *attrs_used = NULL;
>> ! ListCell *lc;
>> ! int i;
>>
>> ! /*
>> ! * First, examine all the attributes needed for joins or final output.
>> ! * Note: we must look at reltargetlist, not the attr_needed data,
>> ! * because attr_needed isn't computed for inheritance child rels.
>> ! */
>> ! pull_varattnos((Node *) rel->reltargetlist, scan_relid, &attrs_used);
>>
>> ! /* Add all the attributes used by restriction clauses. */
>> ! foreach(lc, rel->baserestrictinfo)
>> {
>> ! RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
>> !
>> ! pull_varattnos((Node *) rinfo->clause, scan_relid, &attrs_used);
>> }
>>
>> ! /* Now, are any system columns requested from rel? */
>> ! for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
>> ! {
>> ! if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
>> ! {
>> ! scan_plan->fsSystemCol = true;
>> ! break;
>> ! }
>> ! }
>> !
>> ! bms_free(attrs_used);
>> ! }
>>
>> return scan_plan;
>> }
>
> Would it make sense to call pull_varattnos(reltargetlist), then walk the
> bitmapset and break if we see a system column, then call
> pull_varattnos() on the rinfo->clause? That way, if the targetlist
> request a system column we don't have to walk the RestrictInfos.

Seems like a good idea. Will update the patch.

Best regards,
Etsuro Fujita