From ae025a15988f5491903cd3a2075f308c2773f711 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 26 Aug 2016 16:33:57 +0300 Subject: [PATCH] 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 | 37 +++++++++++++--- .../postgres_fdw/expected/postgres_fdw.out | 43 ++++++++++++++----- contrib/postgres_fdw/postgres_fdw.c | 28 +++++++++++- contrib/postgres_fdw/sql/postgres_fdw.sql | 13 +++++- 4 files changed, 101 insertions(+), 20 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index aaf9108c56..691658f099 100644 --- 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 d7747cc665..d97e694d1a 100644 --- 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 bfd81c46c8..b92f29958f 100644 --- 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 6f684a1b0c..4f68e8904e 100644 --- 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 -- 2.40.0