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>

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index aaf9108c56c34d25a33cb3b400c68db3d3f45a25..691658f099eae3c8db2dbae7f2016f338d8c9ef0 100644 (file)
--- a/contrib/postgres_fdw/deparse.c
+++ b/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
-                    * the remote, since we don't make any effort to ensure
-                    * that local and remote values match (tableoid, in
+                    * System columns other than ctid and oid 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)
+                       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
-    * system columns.
+    * Add ctid and oid if needed.  We currently don't support retrieving any
+    * 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)
    {
        /*

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d7747cc665f801a5f3311e215a56d36ad3d14a71..d97e694d1a4eae1a35ef5301bfbe2260cb610b1c 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/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
 -- ===================================================================
@@ -173,15 +180,16 @@ ALTER FOREIGN TABLE ft2 OPTIONS (schema_name 'S 1', table_name 'T 1');
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
 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)
+                                      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')            | 
+ 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
 -- ===================================================================

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index bfd81c46c8692087336ef68499d92c90bbb5e542..b92f29958f3b7cbe6343b46876ed46c6f586f2fb 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/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);
    }

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 6f684a1b0c3285869d999fd59781ab85d9a4c088..4f68e8904ee0c50a1318b691576c9c107879d6c3 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/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