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.

Best regards,
Etsuro Fujita
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 2097,2106 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
  	RelOptInfo *rel = best_path->path.parent;
  	Index		scan_relid = rel->relid;
  	Oid			rel_oid = InvalidOid;
- 	Bitmapset  *attrs_used = NULL;
  	Plan	   *outer_plan = NULL;
- 	ListCell   *lc;
- 	int			i;
  
  	Assert(rel->fdwroutine != NULL);
  
--- 2097,2103 ----
***************
*** 2169,2204 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
  	}
  
  	/*
! 	 * Detect whether any system columns are requested from rel.  This is a
! 	 * bit of a kluge and might go away someday, so we intentionally leave it
! 	 * out of the API presented to FDWs.
! 	 *
! 	 * 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, rel->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, rel->relid, &attrs_used);
! 	}
  
! 	/* 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;
  		}
- 	}
  
! 	bms_free(attrs_used);
  
  	return scan_plan;
  }
--- 2166,2229 ----
  	}
  
  	/*
! 	 * 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 with relid 0 in the reltargetlist or the
! 	 * restriction clauses, so we skip this in that case.  Note that any such
! 	 * columns in base relations that were joined are assumed to be contained
! 	 * in fdw_scan_tlist.)  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);
  
! 		/* 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;
! 			}
  		}
  
! 		/* Examine all the attributes used by restriction clauses, if needed */
! 		if (!scan_plan->fsSystemCol)
! 		{
! 			foreach(lc, rel->baserestrictinfo)
! 			{
! 				RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
! 
! 				pull_varattnos((Node *) rinfo->clause, scan_relid, &attrs_used);
! 
! 				/* 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;
! 					}
! 				}
! 
! 				if (scan_plan->fsSystemCol)
! 					break;
! 			}
! 		}
! 
! 		bms_free(attrs_used);
! 	}
  
  	return scan_plan;
  }
-- 
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