Thread: [Fwd: Re: dblink patches for comment]
Based on Tom's post today about RC1, it sounds like I need to get this
committed very soon. Any complaints?
Joe
-------- Original Message --------
Subject: Re: [HACKERS] dblink patches for comment
Date: Tue, 02 Jun 2009 16:08:18 -0700
From: Joe Conway <mail@joeconway.com>
Tom Lane wrote:
> The docs patch looks okay, except this comment is a bit hazy:
>
>> + -- Note: local connection must require authentication for this to work properly
>
> I think what it means is
>
>> + -- Note: local connection must require password authentication for this to work properly
>
> If not, please clarify some other way. It might also be good to be a
> bit more clear about what "fail to work properly" might entail.
OK, hopefully the attached is more clear.
> As far as the code goes, hopefully Peter will take a look since he's
> spent more time on the SQL/MED code than I have. The only thing I can
> see that looks bogus is that get_connect_string() is failing to handle
> any quoting/escaping that might be needed for the values to be inserted
> into the connection string. I don't recall offhand what rules libpq
> has for that, but I hope it at least implements doubled single quotes...
Added quote_literal_cstr() around the connection string params. Also
found I needed to restrict the servername string length to NAMEDATALEN
in order to avoid an assert if a full connection string is passed to
dblink_connect().
Other comments?
Thanks,
Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.78
diff -c -r1.78 dblink.c
*** dblink.c 2 Jun 2009 03:21:56 -0000 1.78
--- dblink.c 2 Jun 2009 22:55:42 -0000
***************
*** 46,51 ****
--- 46,52 ----
#include "catalog/pg_type.h"
#include "executor/executor.h"
#include "executor/spi.h"
+ #include "foreign/foreign.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/execnodes.h"
***************
*** 96,101 ****
--- 97,103 ----
static void dblink_connstr_check(const char *connstr);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
+ static char *get_connect_string(const char *servername);
/* Global */
static remoteConn *pconn = NULL;
***************
*** 165,171 ****
} \
else \
{ \
! connstr = conname_or_str; \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
--- 167,177 ----
} \
else \
{ \
! connstr = get_connect_string(conname_or_str); \
! if (connstr == NULL) \
! { \
! connstr = conname_or_str; \
! } \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
***************
*** 210,215 ****
--- 216,222 ----
Datum
dblink_connect(PG_FUNCTION_ARGS)
{
+ char *conname_or_str = NULL;
char *connstr = NULL;
char *connname = NULL;
char *msg;
***************
*** 220,235 ****
if (PG_NARGS() == 2)
{
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
--- 227,247 ----
if (PG_NARGS() == 2)
{
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
+ /* first check for valid foreign data server */
+ connstr = get_connect_string(conname_or_str);
+ if (connstr == NULL)
+ connstr = conname_or_str;
+
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
***************
*** 2353,2355 ****
--- 2365,2426 ----
errcontext("Error occurred on dblink connection named \"%s\": %s.",
dblink_context_conname, dblink_context_msg)));
}
+
+ /*
+ * Obtain connection string for a foreign server
+ */
+ static char *
+ get_connect_string(const char *servername)
+ {
+ ForeignServer *foreign_server = NULL;
+ UserMapping *user_mapping;
+ ListCell *cell;
+ StringInfo buf = makeStringInfo();
+ ForeignDataWrapper *fdw;
+ AclResult aclresult;
+
+ /* first gather the server connstr options */
+ if (strlen(servername) < NAMEDATALEN)
+ foreign_server = GetForeignServerByName(servername, true);
+
+ if (foreign_server)
+ {
+ Oid serverid = foreign_server->serverid;
+ Oid fdwid = foreign_server->fdwid;
+ Oid userid = GetUserId();
+
+ user_mapping = GetUserMapping(userid, serverid);
+ fdw = GetForeignDataWrapper(fdwid);
+
+ /* Check permissions, user must have usage on the server. */
+ aclresult = pg_foreign_server_aclcheck(serverid, userid, ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_FOREIGN_SERVER, foreign_server->servername);
+
+ foreach (cell, fdw->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s=%s ", def->defname, quote_literal_cstr(strVal(def->arg)));
+ }
+
+ foreach (cell, foreign_server->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s=%s ", def->defname, quote_literal_cstr(strVal(def->arg)));
+ }
+
+ foreach (cell, user_mapping->options)
+ {
+
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s=%s ", def->defname, quote_literal_cstr(strVal(def->arg)));
+ }
+
+ return pstrdup(buf->data);
+ }
+ else
+ return NULL;
+ }
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.24
diff -c -r1.24 dblink.out
*** expected/dblink.out 3 Jul 2008 03:56:57 -0000 1.24
--- expected/dblink.out 2 Jun 2009 16:54:37 -0000
***************
*** 784,786 ****
--- 784,829 ----
OK
(1 row)
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ ERROR: password is required
+ DETAIL: Non-superusers must provide a password in the connection string.
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ dblink_connect_u
+ ------------------
+ OK
+ (1 row)
+
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+ a | b | c
+ ----+---+---------------
+ 0 | a | {a0,b0,c0}
+ 1 | b | {a1,b1,c1}
+ 2 | c | {a2,b2,c2}
+ 3 | d | {a3,b3,c3}
+ 4 | e | {a4,b4,c4}
+ 5 | f | {a5,b5,c5}
+ 6 | g | {a6,b6,c6}
+ 7 | h | {a7,b7,c7}
+ 8 | i | {a8,b8,c8}
+ 9 | j | {a9,b9,c9}
+ 10 | k | {a10,b10,c10}
+ (11 rows)
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Index: sql/dblink.sql
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/sql/dblink.sql,v
retrieving revision 1.20
diff -c -r1.20 dblink.sql
*** sql/dblink.sql 6 Apr 2008 16:54:48 -0000 1.20
--- sql/dblink.sql 2 Jun 2009 16:54:37 -0000
***************
*** 364,366 ****
--- 364,391 ----
SELECT dblink_cancel_query('dtest1');
SELECT dblink_error_message('dtest1');
SELECT dblink_disconnect('dtest1');
+
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Index: dblink.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql/doc/src/sgml/dblink.sgml,v
retrieving revision 1.6
diff -c -r1.6 dblink.sgml
*** dblink.sgml 12 Nov 2008 15:52:44 -0000 1.6
--- dblink.sgml 2 Jun 2009 22:54:59 -0000
***************
*** 42,47 ****
--- 42,59 ----
only one unnamed connection is permitted at a time. The connection
will persist until closed or until the database session is ended.
</para>
+
+ <para>
+ The connection string may also be the name of an existing foreign
+ server that utilizes the postgresql_fdw foreign data wrapper library.
+ See the example below, as well as the following:
+ <simplelist type="inline">
+ <member><xref linkend="sql-createforeigndatawrapper" endterm="sql-createforeigndatawrapper-title"></member>
+ <member><xref linkend="sql-createserver" endterm="sql-createserver-title"></member>
+ <member><xref linkend="sql-createusermapping" endterm="sql-createusermapping-title"></member>
+ </simplelist>
+ </para>
+
</refsect1>
<refsect1>
***************
*** 113,118 ****
--- 125,177 ----
----------------
OK
(1 row)
+
+ -- FOREIGN DATA WRAPPER functionality
+ -- Note: local connection must require password authentication for this to work properly
+ -- Otherwise, you will receive the following error from dblink_connect():
+ -- ----------------------------------------------------------------------
+ -- ERROR: password is required
+ -- DETAIL: Non-superuser cannot connect if the server does not request a password.
+ -- HINT: Target server's authentication method must be changed.
+ CREATE USER dblink_regression_test WITH PASSWORD 'secret';
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (hostaddr '127.0.0.1', dbname 'contrib_regression');
+
+ CREATE USER MAPPING FOR dblink_regression_test SERVER fdtest OPTIONS (user 'dblink_regression_test', password
'secret');
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT SELECT ON TABLE foo TO dblink_regression_test;
+
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ SELECT dblink_connect('myconn', 'fdtest');
+ dblink_connect
+ ----------------
+ OK
+ (1 row)
+
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+ a | b | c
+ ----+---+---------------
+ 0 | a | {a0,b0,c0}
+ 1 | b | {a1,b1,c1}
+ 2 | c | {a2,b2,c2}
+ 3 | d | {a3,b3,c3}
+ 4 | e | {a4,b4,c4}
+ 5 | f | {a5,b5,c5}
+ 6 | g | {a6,b6,c6}
+ 7 | h | {a7,b7,c7}
+ 8 | i | {a8,b8,c8}
+ 9 | j | {a9,b9,c9}
+ 10 | k | {a10,b10,c10}
+ (11 rows)
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE SELECT ON TABLE foo FROM dblink_regression_test;
+ DROP USER MAPPING FOR dblink_regression_test SERVER fdtest;
+ DROP USER dblink_regression_test;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
</programlisting>
</refsect1>
</refentry>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Joe Conway <mail@joeconway.com> writes:
> Based on Tom's post today about RC1, it sounds like I need to get this
> committed very soon. Any complaints?
The quoting logic is still completely the wrong thing :-(. For one
thing, quote_literal will try to generate E'' syntax in some cases.
But more to the point, quote_literal's quoting rules don't match
what is needed. A look at libpq's conninfo_parse says that what it
accepts is single-quoted strings in which backslash quotes the next
character. It does not recognize doubled single quotes. I think
you will need to whip up a special-purpose quoting subroutine.
One other really minor point is that the pstrdup here:
> + return pstrdup(buf->data);
is a waste of time. The StringInfo's buffer is already palloc'd.
regards, tom lane
Tom Lane wrote: > The quoting logic is still completely the wrong thing :-(. For one > thing, quote_literal will try to generate E'' syntax in some cases. > But more to the point, quote_literal's quoting rules don't match > what is needed. A look at libpq's conninfo_parse says that what it > accepts is single-quoted strings in which backslash quotes the next > character. It does not recognize doubled single quotes. I think > you will need to whip up a special-purpose quoting subroutine. OK, I see that. I assume I need to care for encoding issues? If so, do I assume server encoding or client encoding? >> + return pstrdup(buf->data); > > is a waste of time. The StringInfo's buffer is already palloc'd. Thanks -- will fix. Joe
Joe Conway <mail@joeconway.com> writes:
> Tom Lane wrote:
>> you will need to whip up a special-purpose quoting subroutine.
> OK, I see that. I assume I need to care for encoding issues? If so, do I
> assume server encoding or client encoding?
Hoo, good point. You can assume the database (server) encoding, because
that's what the local encoding is from the point of view of libpq ---
and the code in conninfo_parse knows nothing of encodings anyway. So
that's a no-op as far as the quoting itself goes. But that reminds me,
weren't you going to add something to force libpq to set client_encoding
to the database encoding?
regards, tom lane
Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Tom Lane wrote:
>>> you will need to whip up a special-purpose quoting subroutine.
>
>> OK, I see that. I assume I need to care for encoding issues? If so, do I
>> assume server encoding or client encoding?
>
> Hoo, good point. You can assume the database (server) encoding, because
> that's what the local encoding is from the point of view of libpq ---
> and the code in conninfo_parse knows nothing of encodings anyway. So
> that's a no-op as far as the quoting itself goes.
OK, got it. I think the attached is what you're looking for, although I
have not yet tested beyond "it compiles" and "it passes make installcheck".
> But that reminds me, weren't you going to add something to force libpq to set client_encoding
> to the database encoding?
Yes, I was going to work that next. I assume it is pretty
straightforward, but I've never been particularly strong on the nuances
of encodings...
Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.78
diff -c -r1.78 dblink.c
*** dblink.c 2 Jun 2009 03:21:56 -0000 1.78
--- dblink.c 6 Jun 2009 18:24:46 -0000
***************
*** 46,52 ****
--- 46,54 ----
#include "catalog/pg_type.h"
#include "executor/executor.h"
#include "executor/spi.h"
+ #include "foreign/foreign.h"
#include "lib/stringinfo.h"
+ #include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "nodes/execnodes.h"
#include "nodes/nodes.h"
***************
*** 96,101 ****
--- 98,105 ----
static void dblink_connstr_check(const char *connstr);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
+ static char *get_connect_string(const char *servername);
+ static char *escape_param_str(const char *from, size_t length, int encoding, size_t *result_len);
/* Global */
static remoteConn *pconn = NULL;
***************
*** 165,171 ****
} \
else \
{ \
! connstr = conname_or_str; \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
--- 169,179 ----
} \
else \
{ \
! connstr = get_connect_string(conname_or_str); \
! if (connstr == NULL) \
! { \
! connstr = conname_or_str; \
! } \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
***************
*** 210,215 ****
--- 218,224 ----
Datum
dblink_connect(PG_FUNCTION_ARGS)
{
+ char *conname_or_str = NULL;
char *connstr = NULL;
char *connname = NULL;
char *msg;
***************
*** 220,235 ****
if (PG_NARGS() == 2)
{
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
--- 229,249 ----
if (PG_NARGS() == 2)
{
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
+ /* first check for valid foreign data server */
+ connstr = get_connect_string(conname_or_str);
+ if (connstr == NULL)
+ connstr = conname_or_str;
+
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
***************
*** 2353,2355 ****
--- 2367,2516 ----
errcontext("Error occurred on dblink connection named \"%s\": %s.",
dblink_context_conname, dblink_context_msg)));
}
+
+ /*
+ * Obtain connection string for a foreign server
+ */
+ static char *
+ get_connect_string(const char *servername)
+ {
+ ForeignServer *foreign_server = NULL;
+ UserMapping *user_mapping;
+ ListCell *cell;
+ StringInfo buf = makeStringInfo();
+ ForeignDataWrapper *fdw;
+ AclResult aclresult;
+
+ /* first gather the server connstr options */
+ if (strlen(servername) < NAMEDATALEN)
+ foreign_server = GetForeignServerByName(servername, true);
+
+ if (foreign_server)
+ {
+ Oid serverid = foreign_server->serverid;
+ Oid fdwid = foreign_server->fdwid;
+ Oid userid = GetUserId();
+ int encoding = GetDatabaseEncoding();
+
+ user_mapping = GetUserMapping(userid, serverid);
+ fdw = GetForeignDataWrapper(fdwid);
+
+ /* Check permissions, user must have usage on the server. */
+ aclresult = pg_foreign_server_aclcheck(serverid, userid, ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_FOREIGN_SERVER, foreign_server->servername);
+
+ foreach (cell, fdw->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg), strlen(strVal(def->arg)), encoding, NULL));
+ }
+
+ foreach (cell, foreign_server->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg), strlen(strVal(def->arg)), encoding, NULL));
+ }
+
+ foreach (cell, user_mapping->options)
+ {
+
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg), strlen(strVal(def->arg)), encoding, NULL));
+ }
+
+ return buf->data;
+ }
+ else
+ return NULL;
+ }
+
+ /*
+ * Escaping libpq connect parameter strings.
+ *
+ * Replaces "'" with "\'" and "\" with "\\".
+ *
+ * length is the length of the source string. (Note: if a terminating NUL
+ * is encountered sooner, stops short of "length"; the behavior
+ * is thus rather like strncpy.)
+ *
+ * For safety the target must be 2*length + 1 bytes long.
+ * A terminating NUL character is added to the output string, whether the
+ * input is NUL-terminated or not.
+ *
+ * Returns the target string.
+ * Optionally returns target length if result_len is non NULL
+ * (not counting the terminating NUL).
+ */
+ static char *
+ escape_param_str(const char *from, size_t length, int encoding, size_t *result_len)
+ {
+ const char *source = from;
+ char *target = (char *) palloc(2*length + 1);
+ char *to = target;
+ size_t remaining = length;
+
+ while (remaining > 0 && *source != '\0')
+ {
+ char c = *source;
+ int len;
+ int i;
+
+ /* Fast path for plain ASCII */
+ if (!IS_HIGHBIT_SET(c))
+ {
+ /* Apply escaping if needed */
+ if (c == '\'' || c == '\\')
+ *target++ = '\\';
+ /* Copy the character */
+ *target++ = c;
+ source++;
+ remaining--;
+ continue;
+ }
+
+ /* Slow path for possible multibyte characters */
+ len = pg_encoding_mblen(encoding, source);
+
+ /* Copy the character */
+ for (i = 0; i < len; i++)
+ {
+ if (remaining == 0 || *source == '\0')
+ break;
+ *target++ = *source++;
+ remaining--;
+ }
+
+ /*
+ * If we hit premature end of string (ie, incomplete multibyte
+ * character), try to pad out to the correct length with spaces. We
+ * may not be able to pad completely, but we will always be able to
+ * insert at least one pad space (since we'd not have quoted a
+ * multibyte character). This should be enough to make a string that
+ * the server will error out on.
+ */
+ if (i < len)
+ {
+ for (; i < len; i++)
+ {
+ if (((size_t) (target - to)) / 2 >= length)
+ break;
+ *target++ = ' ';
+ }
+ break;
+ }
+ }
+
+ /* Write the terminating NUL character. */
+ *target = '\0';
+
+ if (result_len)
+ *result_len = target - to;
+ return to;
+ }
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.24
diff -c -r1.24 dblink.out
*** expected/dblink.out 3 Jul 2008 03:56:57 -0000 1.24
--- expected/dblink.out 2 Jun 2009 16:54:37 -0000
***************
*** 784,786 ****
--- 784,829 ----
OK
(1 row)
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ ERROR: password is required
+ DETAIL: Non-superusers must provide a password in the connection string.
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ dblink_connect_u
+ ------------------
+ OK
+ (1 row)
+
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+ a | b | c
+ ----+---+---------------
+ 0 | a | {a0,b0,c0}
+ 1 | b | {a1,b1,c1}
+ 2 | c | {a2,b2,c2}
+ 3 | d | {a3,b3,c3}
+ 4 | e | {a4,b4,c4}
+ 5 | f | {a5,b5,c5}
+ 6 | g | {a6,b6,c6}
+ 7 | h | {a7,b7,c7}
+ 8 | i | {a8,b8,c8}
+ 9 | j | {a9,b9,c9}
+ 10 | k | {a10,b10,c10}
+ (11 rows)
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Index: sql/dblink.sql
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/sql/dblink.sql,v
retrieving revision 1.20
diff -c -r1.20 dblink.sql
*** sql/dblink.sql 6 Apr 2008 16:54:48 -0000 1.20
--- sql/dblink.sql 2 Jun 2009 16:54:37 -0000
***************
*** 364,366 ****
--- 364,391 ----
SELECT dblink_cancel_query('dtest1');
SELECT dblink_error_message('dtest1');
SELECT dblink_disconnect('dtest1');
+
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Joe Conway <mail@joeconway.com> writes:
> OK, got it. I think the attached is what you're looking for, although I
> have not yet tested beyond "it compiles" and "it passes make installcheck".
You're making it vastly overcomplicated. Just do something like
for (cp = str; *cp; cp++){ if (*cp == '\\' || *cp == '\'') AppendStringInfoChar(buf, '\\');
AppendStringInfoChar(buf,*cp);}
Since you're working in a server-safe encoding, there is no need to
worry about multibyte characters --- the tests will never match
any byte of a multibyte char.
regards, tom lane
Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> OK, got it. I think the attached is what you're looking for, although I
>> have not yet tested beyond "it compiles" and "it passes make installcheck".
>
> You're making it vastly overcomplicated. Just do something like
>
> for (cp = str; *cp; cp++)
> {
> if (*cp == '\\' || *cp == '\'')
> AppendStringInfoChar(buf, '\\');
> AppendStringInfoChar(buf, *cp);
> }
>
> Since you're working in a server-safe encoding, there is no need to
> worry about multibyte characters --- the tests will never match
> any byte of a multibyte char.
It wasn't clear to me that would be safe, but thanks for your patience :-)
This version is clearly simpler. For the record I've included the doc
patch again here. Will commit shortly...
Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.78
diff -c -r1.78 dblink.c
*** dblink.c 2 Jun 2009 03:21:56 -0000 1.78
--- dblink.c 6 Jun 2009 19:08:30 -0000
***************
*** 46,51 ****
--- 46,52 ----
#include "catalog/pg_type.h"
#include "executor/executor.h"
#include "executor/spi.h"
+ #include "foreign/foreign.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/execnodes.h"
***************
*** 96,101 ****
--- 97,104 ----
static void dblink_connstr_check(const char *connstr);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
+ static char *get_connect_string(const char *servername);
+ static char *escape_param_str(const char *from);
/* Global */
static remoteConn *pconn = NULL;
***************
*** 165,171 ****
} \
else \
{ \
! connstr = conname_or_str; \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
--- 168,178 ----
} \
else \
{ \
! connstr = get_connect_string(conname_or_str); \
! if (connstr == NULL) \
! { \
! connstr = conname_or_str; \
! } \
dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
***************
*** 210,215 ****
--- 217,223 ----
Datum
dblink_connect(PG_FUNCTION_ARGS)
{
+ char *conname_or_str = NULL;
char *connstr = NULL;
char *connname = NULL;
char *msg;
***************
*** 220,235 ****
if (PG_NARGS() == 2)
{
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! connstr = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
--- 228,248 ----
if (PG_NARGS() == 2)
{
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(1));
connname = text_to_cstring(PG_GETARG_TEXT_PP(0));
}
else if (PG_NARGS() == 1)
! conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (connname)
rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
sizeof(remoteConn));
+ /* first check for valid foreign data server */
+ connstr = get_connect_string(conname_or_str);
+ if (connstr == NULL)
+ connstr = conname_or_str;
+
/* check password in connection string if not superuser */
dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
***************
*** 2353,2355 ****
--- 2366,2451 ----
errcontext("Error occurred on dblink connection named \"%s\": %s.",
dblink_context_conname, dblink_context_msg)));
}
+
+ /*
+ * Obtain connection string for a foreign server
+ */
+ static char *
+ get_connect_string(const char *servername)
+ {
+ ForeignServer *foreign_server = NULL;
+ UserMapping *user_mapping;
+ ListCell *cell;
+ StringInfo buf = makeStringInfo();
+ ForeignDataWrapper *fdw;
+ AclResult aclresult;
+
+ /* first gather the server connstr options */
+ if (strlen(servername) < NAMEDATALEN)
+ foreign_server = GetForeignServerByName(servername, true);
+
+ if (foreign_server)
+ {
+ Oid serverid = foreign_server->serverid;
+ Oid fdwid = foreign_server->fdwid;
+ Oid userid = GetUserId();
+
+ user_mapping = GetUserMapping(userid, serverid);
+ fdw = GetForeignDataWrapper(fdwid);
+
+ /* Check permissions, user must have usage on the server. */
+ aclresult = pg_foreign_server_aclcheck(serverid, userid, ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_FOREIGN_SERVER, foreign_server->servername);
+
+ foreach (cell, fdw->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg)));
+ }
+
+ foreach (cell, foreign_server->options)
+ {
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg)));
+ }
+
+ foreach (cell, user_mapping->options)
+ {
+
+ DefElem *def = lfirst(cell);
+
+ appendStringInfo(buf, "%s='%s' ", def->defname,
+ escape_param_str(strVal(def->arg)));
+ }
+
+ return buf->data;
+ }
+ else
+ return NULL;
+ }
+
+ /*
+ * Escaping libpq connect parameter strings.
+ *
+ * Replaces "'" with "\'" and "\" with "\\".
+ */
+ static char *
+ escape_param_str(const char *str)
+ {
+ const char *cp;
+ StringInfo buf = makeStringInfo();
+
+ for (cp = str; *cp; cp++)
+ {
+ if (*cp == '\\' || *cp == '\'')
+ appendStringInfoChar(buf, '\\');
+ appendStringInfoChar(buf, *cp);
+ }
+
+ return buf->data;
+ }
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.24
diff -c -r1.24 dblink.out
*** expected/dblink.out 3 Jul 2008 03:56:57 -0000 1.24
--- expected/dblink.out 2 Jun 2009 16:54:37 -0000
***************
*** 784,786 ****
--- 784,829 ----
OK
(1 row)
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ ERROR: password is required
+ DETAIL: Non-superusers must provide a password in the connection string.
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ dblink_connect_u
+ ------------------
+ OK
+ (1 row)
+
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+ a | b | c
+ ----+---+---------------
+ 0 | a | {a0,b0,c0}
+ 1 | b | {a1,b1,c1}
+ 2 | c | {a2,b2,c2}
+ 3 | d | {a3,b3,c3}
+ 4 | e | {a4,b4,c4}
+ 5 | f | {a5,b5,c5}
+ 6 | g | {a6,b6,c6}
+ 7 | h | {a7,b7,c7}
+ 8 | i | {a8,b8,c8}
+ 9 | j | {a9,b9,c9}
+ 10 | k | {a10,b10,c10}
+ (11 rows)
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Index: sql/dblink.sql
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/sql/dblink.sql,v
retrieving revision 1.20
diff -c -r1.20 dblink.sql
*** sql/dblink.sql 6 Apr 2008 16:54:48 -0000 1.20
--- sql/dblink.sql 2 Jun 2009 16:54:37 -0000
***************
*** 364,366 ****
--- 364,391 ----
SELECT dblink_cancel_query('dtest1');
SELECT dblink_error_message('dtest1');
SELECT dblink_disconnect('dtest1');
+
+ -- test foreign data wrapper functionality
+ CREATE USER dblink_regression_test;
+
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
+ CREATE USER MAPPING FOR public SERVER fdtest;
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test;
+
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ -- should fail
+ SELECT dblink_connect('myconn', 'fdtest');
+ -- should succeed
+ SELECT dblink_connect_u('myconn', 'fdtest');
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
+ DROP USER dblink_regression_test;
+ DROP USER MAPPING FOR public SERVER fdtest;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
Index: dblink.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql/doc/src/sgml/dblink.sgml,v
retrieving revision 1.6
diff -c -r1.6 dblink.sgml
*** dblink.sgml 12 Nov 2008 15:52:44 -0000 1.6
--- dblink.sgml 2 Jun 2009 22:54:59 -0000
***************
*** 42,47 ****
--- 42,59 ----
only one unnamed connection is permitted at a time. The connection
will persist until closed or until the database session is ended.
</para>
+
+ <para>
+ The connection string may also be the name of an existing foreign
+ server that utilizes the postgresql_fdw foreign data wrapper library.
+ See the example below, as well as the following:
+ <simplelist type="inline">
+ <member><xref linkend="sql-createforeigndatawrapper" endterm="sql-createforeigndatawrapper-title"></member>
+ <member><xref linkend="sql-createserver" endterm="sql-createserver-title"></member>
+ <member><xref linkend="sql-createusermapping" endterm="sql-createusermapping-title"></member>
+ </simplelist>
+ </para>
+
</refsect1>
<refsect1>
***************
*** 113,118 ****
--- 125,177 ----
----------------
OK
(1 row)
+
+ -- FOREIGN DATA WRAPPER functionality
+ -- Note: local connection must require password authentication for this to work properly
+ -- Otherwise, you will receive the following error from dblink_connect():
+ -- ----------------------------------------------------------------------
+ -- ERROR: password is required
+ -- DETAIL: Non-superuser cannot connect if the server does not request a password.
+ -- HINT: Target server's authentication method must be changed.
+ CREATE USER dblink_regression_test WITH PASSWORD 'secret';
+ CREATE FOREIGN DATA WRAPPER postgresql;
+ CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (hostaddr '127.0.0.1', dbname 'contrib_regression');
+
+ CREATE USER MAPPING FOR dblink_regression_test SERVER fdtest OPTIONS (user 'dblink_regression_test', password
'secret');
+ GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test;
+ GRANT SELECT ON TABLE foo TO dblink_regression_test;
+
+ \set ORIGINAL_USER :USER
+ \c - dblink_regression_test
+ SELECT dblink_connect('myconn', 'fdtest');
+ dblink_connect
+ ----------------
+ OK
+ (1 row)
+
+ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]);
+ a | b | c
+ ----+---+---------------
+ 0 | a | {a0,b0,c0}
+ 1 | b | {a1,b1,c1}
+ 2 | c | {a2,b2,c2}
+ 3 | d | {a3,b3,c3}
+ 4 | e | {a4,b4,c4}
+ 5 | f | {a5,b5,c5}
+ 6 | g | {a6,b6,c6}
+ 7 | h | {a7,b7,c7}
+ 8 | i | {a8,b8,c8}
+ 9 | j | {a9,b9,c9}
+ 10 | k | {a10,b10,c10}
+ (11 rows)
+
+ \c - :ORIGINAL_USER
+ REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
+ REVOKE SELECT ON TABLE foo FROM dblink_regression_test;
+ DROP USER MAPPING FOR dblink_regression_test SERVER fdtest;
+ DROP USER dblink_regression_test;
+ DROP SERVER fdtest;
+ DROP FOREIGN DATA WRAPPER postgresql;
</programlisting>
</refsect1>
</refentry>
Tom Lane wrote:
> But that reminds me, weren't you going to add something to force
> libpq to set client_encoding to the database encoding?
I think the attached is what you had in mind. But I don't know right off
how to trigger the failure (and therefore how to test the solution). A
naive test with two databases, one LATIN2, the other UTF8 does not
produce the error with simple text literals. Any guidance on specific
literals that would trigger the problem?
Thanks,
Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.79
diff -c -r1.79 dblink.c
*** dblink.c 6 Jun 2009 21:27:56 -0000 1.79
--- dblink.c 7 Jun 2009 01:14:44 -0000
***************
*** 48,53 ****
--- 48,54 ----
#include "executor/spi.h"
#include "foreign/foreign.h"
#include "lib/stringinfo.h"
+ #include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "nodes/execnodes.h"
#include "nodes/nodes.h"
***************
*** 185,190 ****
--- 186,192 ----
errdetail("%s", msg))); \
} \
dblink_security_check(conn, rconn); \
+ PQsetClientEncoding(conn, GetDatabaseEncodingName()); \
freeconn = true; \
} \
} while (0)
***************
*** 263,268 ****
--- 265,273 ----
/* check password actually used if not superuser */
dblink_security_check(conn, rconn);
+ /* attempt to set client encoding to match server encoding */
+ PQsetClientEncoding(conn, GetDatabaseEncodingName());
+
if (connname)
{
rconn->conn = conn;
Joe Conway <mail@joeconway.com> writes:
> Tom Lane wrote:
>> But that reminds me, weren't you going to add something to force
>> libpq to set client_encoding to the database encoding?
> I think the attached is what you had in mind.
Looks plausible to me.
> But I don't know right off
> how to trigger the failure (and therefore how to test the solution). A
> naive test with two databases, one LATIN2, the other UTF8 does not
> produce the error with simple text literals. Any guidance on specific
> literals that would trigger the problem?
Hmm, sending some non-ASCII characters from the LATIN2 end to the UTF8
end should do it, I would think. The other direction would probably
not show any immediate error.
regards, tom lane
Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: >> how to trigger the failure (and therefore how to test the solution). A >> naive test with two databases, one LATIN2, the other UTF8 does not >> produce the error with simple text literals. Any guidance on specific >> literals that would trigger the problem? > > Hmm, sending some non-ASCII characters from the LATIN2 end to the UTF8 > end should do it, I would think. The other direction would probably > not show any immediate error. I tried some random high-bit characters on the LATIN2 side, but was not able to stumble upon the right combination of characters to trigger a complaint. I've contacted Ruzsinszky Attila off-list and he said he will get me a self contained test case. Joe
Joe Conway <mail@joeconway.com> writes:
> I think the attached is what you had in mind. But I don't know right off
> how to trigger the failure (and therefore how to test the solution). A
> naive test with two databases, one LATIN2, the other UTF8 does not
> produce the error with simple text literals.
I can reproduce an error (and verify the patch corrects it) using this
test case:
select '�x�y'::text as x;
select * from dblink('dbname = u8', $$select '�x�y'::text$$)
as t1 (x text);
(The two non-ASCII characters are octal 340 and 367, if they don't come
through properly in your mail.) Execute in a LATIN1 database (being sure
client_encoding is also LATIN1), connecting to a database with encoding
UTF8. With the patch, both commands give the same results; without,
I get
ERROR: invalid byte sequence for encoding "UTF8": 0xe078f7
HINT: This error can also happen if the byte sequence does not match the encoding expected by the server, which is
controlledby "client_encoding".
CONTEXT: Error occurred on dblink connection named "unnamed": could not execute query.
Please get this committed soon, we have other stuff to get done
(like a pgindent run).
regards, tom lane
Tom Lane wrote: > Please get this committed soon, we have other stuff to get done > (like a pgindent run). Thanks -- committed. Joe