On 2016/01/15 19:00, Etsuro Fujita wrote:
On 2016/01/12 18:00, Etsuro Fujita wrote:
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.

Done.  Attached is an updated version of the patch.

On second thought, I noticed that detecting whether we see a system column that way needs more cycles in cases where the reltargetlist and the restriction clauses don't contain any system columns. ISTM that such cases are rather common, so I'm inclined to keep that code as-is.

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

Reply via email to