Hi,

PG9.5 allows us to add an oid system column to foreign tables, using
ALTER FOREIGN TABLE SET WITH OIDS, but currently, that column reads as
zeroes in postgres_fdw.  That seems to me like a bug.  So, I'd like to
propose to fix that, by retrieving that column from the remote server
when requested.  I'm attaching a proposed patch for that.

Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 287,298 **** foreign_expr_walker(Node *node,
  					/* Var belongs to foreign table */
  
  					/*
! 					 * System columns other than ctid should not be sent to
! 					 * the remote, since we don't make any effort to ensure
! 					 * that local and remote values match (tableoid, in
  					 * particular, almost certainly doesn't match).
  					 */
  					if (var->varattno < 0 &&
  						var->varattno != SelfItemPointerAttributeNumber)
  						return false;
  
--- 287,299 ----
  					/* Var belongs to foreign table */
  
  					/*
! 					 * System columns other than oid and ctid should not be
! 					 * sent to the remote, since we don't make any effort to
! 					 * ensure that local and remote values match (tableoid, in
  					 * particular, almost certainly doesn't match).
  					 */
  					if (var->varattno < 0 &&
+ 						var->varattno != ObjectIdAttributeNumber &&
  						var->varattno != SelfItemPointerAttributeNumber)
  						return false;
  
***************
*** 911,919 **** deparseTargetList(StringInfo buf,
  	}
  
  	/*
! 	 * Add ctid if needed.  We currently don't support retrieving any other
! 	 * system columns.
  	 */
  	if (bms_is_member(SelfItemPointerAttributeNumber - FirstLowInvalidHeapAttributeNumber,
  					  attrs_used))
  	{
--- 912,936 ----
  	}
  
  	/*
! 	 * Add oid and ctid if needed.  We currently don't support retrieving any
! 	 * other system columns.
  	 */
+ 	if (bms_is_member(ObjectIdAttributeNumber - FirstLowInvalidHeapAttributeNumber,
+ 					  attrs_used))
+ 	{
+ 		if (!first)
+ 			appendStringInfoString(buf, ", ");
+ 		else if (is_returning)
+ 			appendStringInfoString(buf, " RETURNING ");
+ 		first = false;
+ 
+ 		if (qualify_col)
+ 			ADD_REL_QUALIFIER(buf, rtindex);
+ 		appendStringInfoString(buf, "oid");
+ 
+ 		*retrieved_attrs = lappend_int(*retrieved_attrs,
+ 									   ObjectIdAttributeNumber);
+ 	}
  	if (bms_is_member(SelfItemPointerAttributeNumber - FirstLowInvalidHeapAttributeNumber,
  					  attrs_used))
  	{
***************
*** 1467,1474 **** deparseColumnRef(StringInfo buf, int varno, int varattno, PlannerInfo *root,
  {
  	RangeTblEntry *rte;
  
! 	/* varattno can be a whole-row reference, ctid or a regular table column */
! 	if (varattno == SelfItemPointerAttributeNumber)
  	{
  		if (qualify_col)
  			ADD_REL_QUALIFIER(buf, varno);
--- 1484,1500 ----
  {
  	RangeTblEntry *rte;
  
! 	/*
! 	 * varattno can be an oid, a ctid, a whole-row reference, or a regular
! 	 * table column
! 	 */
! 	if (varattno == ObjectIdAttributeNumber)
! 	{
! 		if (qualify_col)
! 			ADD_REL_QUALIFIER(buf, varno);
! 		appendStringInfoString(buf, "oid");
! 	}
! 	else if (varattno == SelfItemPointerAttributeNumber)
  	{
  		if (qualify_col)
  			ADD_REL_QUALIFIER(buf, varno);
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 3744,3749 **** make_tuple_from_result_row(PGresult *res,
--- 3744,3750 ----
  	TupleDesc	tupdesc;
  	Datum	   *values;
  	bool	   *nulls;
+ 	Oid			oid = InvalidOid;
  	ItemPointer ctid = NULL;
  	ConversionLocation errpos;
  	ErrorContextCallback errcallback;
***************
*** 3802,3808 **** make_tuple_from_result_row(PGresult *res,
  		else
  			valstr = PQgetvalue(res, row, j);
  
! 		/* convert value to internal representation */
  		errpos.cur_attno = i;
  		if (i > 0)
  		{
--- 3803,3813 ----
  		else
  			valstr = PQgetvalue(res, row, j);
  
! 		/*
! 		 * convert value to internal representation
! 		 *
! 		 * Note: we ignore system columns other than oid and ctid in result
! 		 */
  		errpos.cur_attno = i;
  		if (i > 0)
  		{
***************
*** 3815,3823 **** make_tuple_from_result_row(PGresult *res,
  											  attinmeta->attioparams[i - 1],
  											  attinmeta->atttypmods[i - 1]);
  		}
  		else if (i == SelfItemPointerAttributeNumber)
  		{
! 			/* ctid --- note we ignore any other system column in result */
  			if (valstr != NULL)
  			{
  				Datum		datum;
--- 3820,3839 ----
  											  attinmeta->attioparams[i - 1],
  											  attinmeta->atttypmods[i - 1]);
  		}
+ 		else if (i == ObjectIdAttributeNumber)
+ 		{
+ 			/* oid */
+ 			if (valstr != NULL)
+ 			{
+ 				Datum		datum;
+ 
+ 				datum = DirectFunctionCall1(oidin, CStringGetDatum(valstr));
+ 				oid = DatumGetObjectId(datum);
+ 			}
+ 		}
  		else if (i == SelfItemPointerAttributeNumber)
  		{
! 			/* ctid */
  			if (valstr != NULL)
  			{
  				Datum		datum;
***************
*** 3849,3854 **** make_tuple_from_result_row(PGresult *res,
--- 3865,3876 ----
  	tuple = heap_form_tuple(tupdesc, values, nulls);
  
  	/*
+ 	 * If we have an OID to return, install it.
+ 	 */
+ 	if (OidIsValid(oid))
+ 		HeapTupleSetOid(tuple, oid);
+ 
+ 	/*
  	 * If we have a CTID to return, install it in both t_self and t_ctid.
  	 * t_self is the normal place, but if the tuple is converted to a
  	 * composite Datum, t_self will be lost; setting t_ctid allows CTID to be
***************
*** 3881,3886 **** conversion_error_callback(void *arg)
--- 3903,3910 ----
  
  		if (errpos->cur_attno > 0 && errpos->cur_attno <= tupdesc->natts)
  			attname = NameStr(tupdesc->attrs[errpos->cur_attno - 1]->attname);
+ 		else if (errpos->cur_attno == ObjectIdAttributeNumber)
+ 			attname = "oid";
  		else if (errpos->cur_attno == SelfItemPointerAttributeNumber)
  			attname = "ctid";
  
-- 
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