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