Support OID system column in postgres_fdw.

You can use ALTER FOREIGN TABLE SET WITH OIDS on a foreign table, but the
oid column read out as zeros, because the postgres_fdw didn't know about
it. Teach postgres_fdw how to fetch it.

Etsuro Fujita, with an additional test case by me.

Discussion: <56E90A76.5000503@lab.ntt.co.jp>

contrib/postgres_fdw/deparse.c

@@ -287,13 +287,14 @@ foreign_expr_walker(Node *node,
 					/* Var belongs to foreign table */
 
 					/*
-					 * System columns other than ctid should not be sent to
+					 * System columns other than ctid and oid should not be
-					 * the remote, since we don't make any effort to ensure
+					 * sent to the remote, since we don't make any effort to
-					 * that local and remote values match (tableoid, in
+					 * ensure that local and remote values match (tableoid, in
 					 * particular, almost certainly doesn't match).
 					 */
 					if (var->varattno < 0 &&
-						var->varattno != SelfItemPointerAttributeNumber)
+						var->varattno != SelfItemPointerAttributeNumber &&
+						var->varattno != ObjectIdAttributeNumber)
 						return false;
 
 					/* Else check the collation */
@@ -913,8 +914,8 @@ deparseTargetList(StringInfo buf,
 	}
 
 	/*
-	 * Add ctid if needed.  We currently don't support retrieving any other
+	 * Add ctid and oid if needed.  We currently don't support retrieving any
-	 * system columns.
+	 * other system columns.
 	 */
 	if (bms_is_member(SelfItemPointerAttributeNumber - FirstLowInvalidHeapAttributeNumber,
 					  attrs_used))
@@ -932,6 +933,22 @@ deparseTargetList(StringInfo buf,
 		*retrieved_attrs = lappend_int(*retrieved_attrs,
 									   SelfItemPointerAttributeNumber);
 	}
+	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);
+	}
 
 	/* Don't generate bad syntax if no undropped columns */
 	if (first && !is_returning)
@@ -1574,13 +1591,19 @@ deparseColumnRef(StringInfo buf, int varno, int varattno, PlannerInfo *root,
 {
 	RangeTblEntry *rte;
 
+	/* We support fetching the remote side's CTID and OID. */
 	if (varattno == SelfItemPointerAttributeNumber)
 	{
-		/* We support fetching the remote side's CTID. */
 		if (qualify_col)
 			ADD_REL_QUALIFIER(buf, varno);
 		appendStringInfoString(buf, "ctid");
 	}
+	else if (varattno == ObjectIdAttributeNumber)
+	{
+		if (qualify_col)
+			ADD_REL_QUALIFIER(buf, varno);
+		appendStringInfoString(buf, "oid");
+	}
 	else if (varattno < 0)
 	{
 		/*

contrib/postgres_fdw/expected/postgres_fdw.out

@@ -124,6 +124,13 @@ CREATE FOREIGN TABLE ft6 (
 	c2 int NOT NULL,
 	c3 text
 ) SERVER loopback2 OPTIONS (schema_name 'S 1', table_name 'T 4');
+-- A table with oids. CREATE FOREIGN TABLE doesn't support the
+-- WITH OIDS option, but ALTER does.
+CREATE FOREIGN TABLE ft_pg_type (
+	typname name,
+	typlen smallint
+) SERVER loopback OPTIONS (schema_name 'pg_catalog', table_name 'pg_type');
+ALTER TABLE ft_pg_type SET WITH OIDS;
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
@@ -175,13 +182,14 @@ ALTER FOREIGN TABLE ft2 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
 \det+
                                       List of foreign tables
  Schema |   Table    |  Server   |                   FDW Options                    | Description 
---------+-------+-----------+---------------------------------------+-------------
+--------+------------+-----------+--------------------------------------------------+-------------
  public | ft1        | loopback  | (schema_name 'S 1', table_name 'T 1')            | 
  public | ft2        | loopback  | (schema_name 'S 1', table_name 'T 1')            | 
  public | ft4        | loopback  | (schema_name 'S 1', table_name 'T 3')            | 
  public | ft5        | loopback  | (schema_name 'S 1', table_name 'T 4')            | 
  public | ft6        | loopback2 | (schema_name 'S 1', table_name 'T 4')            | 
-(5 rows)
+ public | ft_pg_type | loopback  | (schema_name 'pg_catalog', table_name 'pg_type') | 
+(6 rows)
 
 -- Now we should be able to run ANALYZE.
 -- To exercise multiple code paths, we use local stats on ft1
@@ -2485,7 +2493,7 @@ DEALLOCATE st2;
 DEALLOCATE st3;
 DEALLOCATE st4;
 DEALLOCATE st5;
--- System columns, except ctid, should not be sent to remote
+-- System columns, except ctid and oid, should not be sent to remote
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
                                   QUERY PLAN                                   
@@ -2553,6 +2561,21 @@ SELECT ctid, * FROM ft1 t1 LIMIT 1;
  (0,1) |  1 |  1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1          | foo
 (1 row)
 
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT oid, * FROM ft_pg_type WHERE typname = 'int4';
+                                             QUERY PLAN                                             
+----------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft_pg_type
+   Output: oid, typname, typlen
+   Remote SQL: SELECT typname, typlen, oid FROM pg_catalog.pg_type WHERE ((typname = 'int4'::name))
+(3 rows)
+
+SELECT oid, * FROM ft_pg_type WHERE typname = 'int4';
+ oid | typname | typlen 
+-----+---------+--------
+  23 | int4    |      4
+(1 row)
+
 -- ===================================================================
 -- used in pl/pgsql function
 -- ===================================================================

contrib/postgres_fdw/postgres_fdw.c

@@ -4374,6 +4374,7 @@ make_tuple_from_result_row(PGresult *res,
 	Datum	   *values;
 	bool	   *nulls;
 	ItemPointer ctid = NULL;
+	Oid			oid = InvalidOid;
 	ConversionLocation errpos;
 	ErrorContextCallback errcallback;
 	MemoryContext oldcontext;
@@ -4431,7 +4432,11 @@ make_tuple_from_result_row(PGresult *res,
 		else
 			valstr = PQgetvalue(res, row, j);
 
-		/* convert value to internal representation */
+		/*
+		 * convert value to internal representation
+		 *
+		 * Note: we ignore system columns other than ctid and oid in result
+		 */
 		errpos.cur_attno = i;
 		if (i > 0)
 		{
@@ -4446,7 +4451,7 @@ make_tuple_from_result_row(PGresult *res,
 		}
 		else if (i == SelfItemPointerAttributeNumber)
 		{
-			/* ctid --- note we ignore any other system column in result */
+			/* ctid */
 			if (valstr != NULL)
 			{
 				Datum		datum;
@@ -4455,6 +4460,17 @@ make_tuple_from_result_row(PGresult *res,
 				ctid = (ItemPointer) DatumGetPointer(datum);
 			}
 		}
+		else if (i == ObjectIdAttributeNumber)
+		{
+			/* oid */
+			if (valstr != NULL)
+			{
+				Datum		datum;
+
+				datum = DirectFunctionCall1(oidin, CStringGetDatum(valstr));
+				oid = DatumGetObjectId(datum);
+			}
+		}
 		errpos.cur_attno = 0;
 
 		j++;
@@ -4498,6 +4514,12 @@ make_tuple_from_result_row(PGresult *res,
 	HeapTupleHeaderSetXmin(tuple->t_data, InvalidTransactionId);
 	HeapTupleHeaderSetCmin(tuple->t_data, InvalidTransactionId);
 
+	/*
+	 * If we have an OID to return, install it.
+	 */
+	if (OidIsValid(oid))
+		HeapTupleSetOid(tuple, oid);
+
 	/* Clean up */
 	MemoryContextReset(temp_context);
 
@@ -4525,6 +4547,8 @@ conversion_error_callback(void *arg)
 			attname = NameStr(tupdesc->attrs[errpos->cur_attno - 1]->attname);
 		else if (errpos->cur_attno == SelfItemPointerAttributeNumber)
 			attname = "ctid";
+		else if (errpos->cur_attno == ObjectIdAttributeNumber)
+			attname = "oid";
 
 		relname = RelationGetRelationName(errpos->rel);
 	}

contrib/postgres_fdw/sql/postgres_fdw.sql

@@ -136,6 +136,14 @@ CREATE FOREIGN TABLE ft6 (
 	c3 text
 ) SERVER loopback2 OPTIONS (schema_name 'S 1', table_name 'T 4');
 
+-- A table with oids. CREATE FOREIGN TABLE doesn't support the
+-- WITH OIDS option, but ALTER does.
+CREATE FOREIGN TABLE ft_pg_type (
+	typname name,
+	typlen smallint
+) SERVER loopback OPTIONS (schema_name 'pg_catalog', table_name 'pg_type');
+ALTER TABLE ft_pg_type SET WITH OIDS;
+
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
@@ -577,7 +585,7 @@ DEALLOCATE st3;
 DEALLOCATE st4;
 DEALLOCATE st5;
 
--- System columns, except ctid, should not be sent to remote
+-- System columns, except ctid and oid, should not be sent to remote
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
 SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
@@ -590,6 +598,9 @@ SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT ctid, * FROM ft1 t1 LIMIT 1;
 SELECT ctid, * FROM ft1 t1 LIMIT 1;
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT oid, * FROM ft_pg_type WHERE typname = 'int4';
+SELECT oid, * FROM ft_pg_type WHERE typname = 'int4';
 
 -- ===================================================================
 -- used in pl/pgsql function