Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

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.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services