> On Fri, Oct 25, 2019 at 12:38 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > end of things. And allowing arbitrary queries to go over a postgres_fdw > > connection would be absolutely disastrous from a debuggability and > > maintainability standpoint, because they might change the remote > > session's state in ways that postgres_fdw isn't prepared to handle. > > (In a dblink connection, the remote session's state is the user's > > responsibility to manage, but this isn't the case for postgres_fdw.) > > So I think this proposal has to be firmly rejected.
On Mon, Oct 28, 2019 at 1:54 PM Robert Haas <robertmh...@gmail.com> wrote: > I think the reduction in debuggability and maintainability has to be > balanced against a possible significant gain in usability. I mean, > you could document that if the values of certain GUCs are changed, or > if you create and drop prepared statements with certain names, it > might cause queries in the same session issued through the regular > foreign table API to produce wrong answers. That would still leave an > enormous number of queries that users could issue with absolutely no > problems. I understand both points, the alternatives and the tradeoffs. My motivations not use dblink are twofold: to purposefully reuse the connection pool in postgres_fdw, and to avoid installing another extension. I cannot speak to whether this can be advantageous to others to accept the tradeoffs. If you are still interested, I'm willing to listen to the feedback and continue improving the patch. Otherwise we can settle it here and (of course!) I won't take any offense because of that. Find attached v2 of the patch with the following changes: - added support for commands, as it failed upon PGRES_COMMAND_OK (with tests with prepared statements) - documentation for the new function, with the mentioned caveats - removed the test with the `SELECT current_user`, because it produced different results depending on the execution environment. Regards -Rafa
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile index 85394b4f1f..85a4ecb900 100644 --- a/contrib/postgres_fdw/Makefile +++ b/contrib/postgres_fdw/Makefile @@ -8,7 +8,7 @@ PG_CPPFLAGS = -I$(libpq_srcdir) SHLIB_LINK_INTERNAL = $(libpq) EXTENSION = postgres_fdw -DATA = postgres_fdw--1.0.sql +DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1.sql REGRESS = postgres_fdw diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 88dbaa2493..2219235731 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -8801,3 +8801,72 @@ SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 -- Clean-up RESET enable_partitionwise_aggregate; +-- =================================================================== +-- test postgres_fdw_query(server name, sql text) +-- =================================================================== +-- Most simple SELECT through postgres_fdw_query +SELECT * FROM postgres_fdw_query('loopback', 'SELECT 42') AS t(i int); + i +---- + 42 +(1 row) + +-- Select schemas owned by the role configured in the user mapping +SELECT * FROM postgres_fdw_query('loopback', $$SELECT s.nspname + FROM pg_catalog.pg_namespace s + JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner + WHERE u.usename = current_user + ORDER BY s.nspname$$ +) AS schemas(schema_name name); + schema_name +-------------------- + S 1 + import_dest1 + import_dest2 + import_dest3 + import_dest4 + import_dest5 + import_source + information_schema + pg_catalog + pg_temp_1 + pg_toast + pg_toast_temp_1 + public +(13 rows) + +-- Select tables and views in a given foreign schema that the role +-- configured in the user mapping has access to +SELECT * FROM postgres_fdw_query('loopback', $$SELECT table_name, table_type + FROM information_schema.tables + WHERE table_schema = 'S 1' + ORDER BY table_name$$ +) AS schemas(table_name text, table_type text); + table_name | table_type +------------+------------ + T 1 | BASE TABLE + T 2 | BASE TABLE + T 3 | BASE TABLE + T 4 | BASE TABLE +(4 rows) + +-- Test we can send commands (e.g: prepared statements) +SELECT * FROM postgres_fdw_query('loopback', $$PREPARE fooplan (int) AS +SELECT $1 + 42$$) AS t(res text); + res +--------- + PREPARE +(1 row) + +SELECT * FROM postgres_fdw_query('loopback', 'EXECUTE fooplan (1)') AS t(i int); + i +---- + 43 +(1 row) + +SELECT * FROM postgres_fdw_query('loopback', 'DEALLOCATE fooplan') AS t(res text); + res +------------ + DEALLOCATE +(1 row) + diff --git a/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql new file mode 100644 index 0000000000..15a7c83519 --- /dev/null +++ b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql @@ -0,0 +1,7 @@ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION postgres_fdw" to load this file. \quit + +CREATE FUNCTION postgres_fdw_query(server name, sql text) +RETURNS SETOF record +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; diff --git a/contrib/postgres_fdw/postgres_fdw--1.1.sql b/contrib/postgres_fdw/postgres_fdw--1.1.sql new file mode 100644 index 0000000000..1f4dd1f32b --- /dev/null +++ b/contrib/postgres_fdw/postgres_fdw--1.1.sql @@ -0,0 +1,21 @@ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION postgres_fdw" to load this file. \quit + +CREATE FUNCTION postgres_fdw_handler() +RETURNS fdw_handler +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; + +CREATE FUNCTION postgres_fdw_validator(text[], oid) +RETURNS void +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; + +CREATE FUNCTION postgres_fdw_query(server name, sql text) +RETURNS SETOF record +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; + +CREATE FOREIGN DATA WRAPPER postgres_fdw + HANDLER postgres_fdw_handler + VALIDATOR postgres_fdw_validator; diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 025f922b4c..ea88da9209 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -5900,3 +5900,200 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel) /* We didn't find any suitable equivalence class expression */ return NULL; } + + + +static void prepTuplestoreResult(FunctionCallInfo fcinfo); + +PG_FUNCTION_INFO_V1(postgres_fdw_query); + +Datum +postgres_fdw_query(PG_FUNCTION_ARGS) +{ + ReturnSetInfo *rsinfo; + Name server_name; + text *sql_text; + char *server; + char *sql; + Oid userid; + PGconn *conn; + UserMapping *user_mapping; + ForeignServer *foreign_server; + PGresult *res = NULL; + TupleDesc tupdesc; + int ntuples; + int nfields; + bool is_sql_cmd; + + prepTuplestoreResult(fcinfo); + rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + /* One-time setup code appears here: */ + + // Get input args + server_name = PG_GETARG_NAME(0); + sql_text = PG_GETARG_TEXT_PP(1); + + server = NameStr(*server_name); + sql = text_to_cstring(sql_text); + + elog(DEBUG3, "server = %s", server); + elog(DEBUG3, "sql = %s", sql); + + // Get a connection to the server with the current user + userid = GetUserId(); + foreign_server = GetForeignServerByName(server, false); + user_mapping = GetUserMapping(userid, foreign_server->serverid); + conn = GetConnection(user_mapping, false); + + // Execute the sql query + PG_TRY(); + { + res = pgfdw_exec_query(conn, sql); + + if (PQresultStatus(res) == PGRES_COMMAND_OK) + { + is_sql_cmd = true; + + /* + * need a tuple descriptor representing one TEXT column to return + * the command status string as our result tuple + */ + tupdesc = CreateTemplateTupleDesc(1, false); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "status", + TEXTOID, -1, 0); + ntuples = 1; + nfields = 1; + } + else + { + is_sql_cmd = false; + + if (PQresultStatus(res) != PGRES_TUPLES_OK) + pgfdw_report_error(ERROR, res, conn, false, sql); + + nfields = PQnfields(res); + + /* get a tuple descriptor for our result type */ + switch (get_call_result_type(fcinfo, NULL, &tupdesc)) + { + case TYPEFUNC_COMPOSITE: + /* success */ + break; + case TYPEFUNC_RECORD: + /* failed to determine actual type of RECORD */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("function returning record called in context " + "that cannot accept type record"))); + break; + default: + /* result type isn't composite */ + elog(ERROR, "return type must be a row type"); + break; + } + + /* make sure we have a persistent copy of the tupdesc */ + tupdesc = CreateTupleDescCopy(tupdesc); + ntuples = PQntuples(res); + nfields = PQnfields(res); + } + + /* check result and tuple descriptor have the same number of columns */ + if (nfields != tupdesc->natts) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("remote query result rowtype does not match " + "the specified FROM clause rowtype"))); + + if (ntuples > 0) + { + AttInMetadata *attinmeta; + Tuplestorestate *tupstore; + MemoryContext oldcontext; + int row; + char **values; + + attinmeta = TupleDescGetAttInMetadata(tupdesc); + + oldcontext = MemoryContextSwitchTo( + rsinfo->econtext->ecxt_per_query_memory); + tupstore = tuplestore_begin_heap(true, false, work_mem); + rsinfo->setResult = tupstore; + rsinfo->setDesc = tupdesc; + MemoryContextSwitchTo(oldcontext); + + values = (char **) palloc(nfields * sizeof(char *)); + + /* put all tuples into the tuplestore */ + for (row = 0; row < ntuples; row++) + { + HeapTuple tuple; + + if (is_sql_cmd) + { + values[0] = PQcmdStatus(res); + } + else + { + int i; + + for (i = 0; i < nfields; i++) + { + if (PQgetisnull(res, row, i)) + values[i] = NULL; + else + values[i] = PQgetvalue(res, row, i); + } + } + + /* build the tuple and put it into the tuplestore. */ + tuple = BuildTupleFromCStrings(attinmeta, values); + tuplestore_puttuple(tupstore, tuple); + } + + /* clean up and return the tuplestore */ + tuplestore_donestoring(tupstore); + } + + PQclear(res); + } + PG_CATCH(); + { + if (res) + PQclear(res); + PG_RE_THROW(); + } + PG_END_TRY(); + + ReleaseConnection(conn); + return (Datum) 0; +} + +/* + * Verify function caller can handle a tuplestore result, and set up for that. + * + * Note: if the caller returns without actually creating a tuplestore, the + * executor will treat the function result as an empty set. + */ +static void +prepTuplestoreResult(FunctionCallInfo fcinfo) +{ + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + + /* check to see if query supports us returning a tuplestore */ + if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsinfo->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not allowed in this context"))); + + /* let the executor know we're sending back a tuplestore */ + rsinfo->returnMode = SFRM_Materialize; + + /* caller must fill these to return a non-empty result */ + rsinfo->setResult = NULL; + rsinfo->setDesc = NULL; +} diff --git a/contrib/postgres_fdw/postgres_fdw.control b/contrib/postgres_fdw/postgres_fdw.control index f9ed490752..d489382064 100644 --- a/contrib/postgres_fdw/postgres_fdw.control +++ b/contrib/postgres_fdw/postgres_fdw.control @@ -1,5 +1,5 @@ # postgres_fdw extension comment = 'foreign-data wrapper for remote PostgreSQL servers' -default_version = '1.0' +default_version = '1.1' module_pathname = '$libdir/postgres_fdw' relocatable = true diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index a87c57df7b..d62fc229f6 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2407,3 +2407,33 @@ SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 -- Clean-up RESET enable_partitionwise_aggregate; + + +-- =================================================================== +-- test postgres_fdw_query(server name, sql text) +-- =================================================================== + +-- Most simple SELECT through postgres_fdw_query +SELECT * FROM postgres_fdw_query('loopback', 'SELECT 42') AS t(i int); + +-- Select schemas owned by the role configured in the user mapping +SELECT * FROM postgres_fdw_query('loopback', $$SELECT s.nspname + FROM pg_catalog.pg_namespace s + JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner + WHERE u.usename = current_user + ORDER BY s.nspname$$ +) AS schemas(schema_name name); + +-- Select tables and views in a given foreign schema that the role +-- configured in the user mapping has access to +SELECT * FROM postgres_fdw_query('loopback', $$SELECT table_name, table_type + FROM information_schema.tables + WHERE table_schema = 'S 1' + ORDER BY table_name$$ +) AS schemas(table_name text, table_type text); + +-- Test we can send commands (e.g: prepared statements) +SELECT * FROM postgres_fdw_query('loopback', $$PREPARE fooplan (int) AS +SELECT $1 + 42$$) AS t(res text); +SELECT * FROM postgres_fdw_query('loopback', 'EXECUTE fooplan (1)') AS t(i int); +SELECT * FROM postgres_fdw_query('loopback', 'DEALLOCATE fooplan') AS t(res text); diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 737336f651..c3bc002d24 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -443,7 +443,7 @@ </sect3> </sect2> - <sect2> + <sect2 id="postgres-fdw-connection-management"> <title>Connection Management</title> <para> @@ -528,7 +528,7 @@ </para> </sect2> - <sect2> + <sect2 id="postgres-fdw-execution-environment"> <title>Remote Query Execution Environment</title> <para> @@ -584,6 +584,185 @@ </para> </sect2> + <sect2> + <title>Remote Execution of Arbitrary Queries</title> + + <para> + In some instances it may be useful to ship queries to a remote server not + depending upon the local planning on a foreign table. To that end, <xref + linkend="postgres-fdw-query"/> can be used. + </para> + <para> + Mind that <emphasis>it is the user's responsibility</emphasis> to ensure + session state is not affected by queries sent through + <function>postgres_fdw_query()</function> to the remote server. + </para> + + <refentry id="postgres-fdw-query" xreflabel="postgres_fdw_query"> + <indexterm> + <primary>postgres-fdw</primary> + </indexterm> + + <refmeta> + <refentrytitle>postgres_fdw_query</refentrytitle> + <manvolnum>3</manvolnum> + </refmeta> + + <refnamediv> + <refname>postgres_fdw_query</refname> + <refpurpose>executes a query in a foreign server</refpurpose> + </refnamediv> + + <refsynopsisdiv> +<synopsis> +postgres_fdw_query(server name, sql text) returns setof record +</synopsis> + </refsynopsisdiv> + + <refsect1> + <title>Description</title> + + <para> + <function>postgres_fdw_query</function> executes a query (usually a <command>SELECT</command>, + but it can be any SQL statement that returns rows) in a foreign server. + </para> + </refsect1> + + <refsect1> + <title>Arguments</title> + + <variablelist> + <varlistentry> + <term><parameter>server</parameter></term> + <listitem> + <para> + Name of the foreign server to use, defined with + <command>CREATE SERVER</command> and accessed with the applicable user + mapping. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><parameter>sql</parameter></term> + <listitem> + <para> + The SQL query that you wish to execute in the foreign server, + for example <literal>SELECT * FROM foo</literal>. + </para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1> + <title>Return Value</title> + + <para> + The function returns the row(s) produced by the query. Since + <function>postgres_fdw_query</function> can be used with any query, it is + declared to return <type>record</type>, rather than specifying any + particular set of columns. This means that you must specify the expected + set of columns in the calling query — otherwise + <productname>PostgreSQL</productname> would not know what to expect. Here + is an example: + +<programlisting> +SELECT * FROM postgres_fdw_query('foreign_server', $$SELECT table_name, +table_type + FROM information_schema.tables + WHERE table_schema = 'public' + ORDER BY table_name$$ +) AS schemas(table_name text, table_type text); +</programlisting> + + The <quote>alias</quote> part of the <literal>FROM</literal> clause must + specify the column names and types that the function will return. + </para> + </refsect1> + + <refsect1> + <title>Caveats</title> + + <para> + The function <function>postgres_fdw_query</function> does not perform any + checks on the input <parameter>sql</parameter>. + </para> + + <para> + It also has the potential to change the state of the remote session. In + particular, it is recommended <emphasis>not</emphasis> to change + session-level settings and avoid using prepared statements prefixed with + <literal>pgsql_fdw_prep_</literal>. Otherwise it can interfere with the + regular functioning of <literal>postgres_fdw</literal>. It it is the + caller's responsibility not to interfere the session state managed by + <literal>postgres_fdw</literal>. + </para> + + <para> + Please see <xref linkend="postgres-fdw-connection-management" /> and <xref + linkend="postgres-fdw-execution-environment" /> to understand how + connections and sessions are managed by <literal>postgres_fdw</literal>. + </para> + + <para> + As an alternative, the older <xref linkend="dblink"/> module offers + similar functionality and sessions are managed independently from + <literal>postgres_fdw</literal>. + </para> + + </refsect1> + + <refsect1> + <title>Examples</title> + +<screen> +-- Simple query +SELECT * FROM postgres_fdw_query('loopback', 'SELECT 42') AS t(i int); + i +---- + 42 +(1 row) + +-- Retrieve info from the foreign information_schema +SELECT * FROM postgres_fdw_query('loopback', $$SELECT table_name, table_type + FROM information_schema.tables + WHERE table_schema = 'S 1' + ORDER BY table_name$$ +) AS schemas(table_name text, table_type text); + table_name | table_type +------------+------------ + T 1 | BASE TABLE + T 2 | BASE TABLE + T 3 | BASE TABLE + T 4 | BASE TABLE +(4 rows) + +-- Prepared statements +SELECT * FROM postgres_fdw_query('loopback', $$PREPARE fooplan (int) AS +SELECT $1 + 42$$) AS t(res text); + res +--------- + PREPARE +(1 row) + +SELECT * FROM postgres_fdw_query('loopback', 'EXECUTE fooplan (1)') AS t(i int); + i +---- + 43 +(1 row) + +SELECT * FROM postgres_fdw_query('loopback', 'DEALLOCATE fooplan') AS t(res text); + res +------------ + DEALLOCATE +(1 row) +</screen> + </refsect1> + </refentry> + + </sect2> + <sect2> <title>Cross-Version Compatibility</title>