Tom Lane wrote:
Joe Conway <[EMAIL PROTECTED]> writes:
[ improved patch ]
Still a couple quibbles:
[ more good feedback ]
All valid complaints, and noticeably improved/simplified code as a
result. Third patch attached.
Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.73
diff -c -r1.73 dblink.c
*** dblink.c 4 Apr 2008 17:02:56 -0000 1.73
--- dblink.c 1 Jun 2008 23:52:39 -0000
***************
*** 94,99 ****
--- 94,100 ----
static Oid get_relid_from_relname(text *relname_text);
static char *generate_relation_name(Oid relid);
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);
/* Global */
static remoteConn *pconn = NULL;
***************
*** 125,158 ****
} \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
! do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! elog(ERROR, "%s: %s", p2, msg); \
! } while (0)
!
! #define DBLINK_RES_ERROR(p2) \
do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! ereport(ERROR, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg("%s", p2), \
! errdetail("%s", msg))); \
} while (0)
! #define DBLINK_RES_ERROR_AS_NOTICE(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! ereport(NOTICE, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg("%s", p2), \
! errdetail("%s", msg))); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
--- 126,145 ----
} \
} while (0)
! #define xpstrdup(var_c, var_) \
do { \
! if (var_ != NULL) \
! var_c = pstrdup(var_); \
! else \
! var_c = NULL; \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! elog(ERROR, "%s: %s", p2, msg); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
***************
*** 396,408 ****
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR("sql error");
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE("sql error");
! PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
! }
}
PQclear(res);
--- 383,390 ----
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, "could not open cursor", fail);
! PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
}
PQclear(res);
***************
*** 470,482 ****
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR("sql error");
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE("sql error");
! PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
! }
}
PQclear(res);
--- 452,459 ----
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, "could not close cursor", fail);
! PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
}
PQclear(res);
***************
*** 513,519 ****
int call_cntr;
int max_calls;
AttInMetadata *attinmeta;
- char *msg;
PGresult *res = NULL;
MemoryContext oldcontext;
char *conname = NULL;
--- 490,495 ----
***************
*** 590,602 ****
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR("sql error");
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE("sql error");
! SRF_RETURN_DONE(funcctx);
! }
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
--- 566,573 ----
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, "could not fetch from cursor", fail);
! SRF_RETURN_DONE(funcctx);
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
***************
*** 846,860 ****
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR("sql error");
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE("sql error");
! if (freeconn)
! PQfinish(conn);
! SRF_RETURN_DONE(funcctx);
! }
}
if (PQresultStatus(res) == PGRES_COMMAND_OK)
--- 817,826 ----
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, "could not execute query", fail);
! if (freeconn)
! PQfinish(conn);
! SRF_RETURN_DONE(funcctx);
}
if (PQresultStatus(res) == PGRES_COMMAND_OK)
***************
*** 1180,1189 ****
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR("sql error");
! else
! DBLINK_RES_ERROR_AS_NOTICE("sql error");
/* need a tuple descriptor representing one TEXT column */
tupdesc = CreateTemplateTupleDesc(1, false);
--- 1146,1152 ----
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, "could not execute command", fail);
/* need a tuple descriptor representing one TEXT column */
tupdesc = CreateTemplateTupleDesc(1, false);
***************
*** 1195,1201 ****
* result tuple
*/
sql_cmd_status = cstring_to_text("ERROR");
-
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
--- 1158,1163 ----
***************
*** 2288,2290 ****
--- 2250,2303 ----
}
}
}
+
+ static void
+ dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail)
+ {
+ int level;
+ char *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+ char *pg_diag_message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
+ char *pg_diag_message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
+ char *pg_diag_message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
+ char *pg_diag_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
+ int sqlstate;
+ char *message_primary;
+ char *message_detail;
+ char *message_hint;
+ char *message_context;
+ const char *dblink_context_conname = "unnamed";
+
+ if (fail)
+ level = ERROR;
+ else
+ level = NOTICE;
+
+ if (pg_diag_sqlstate)
+ sqlstate = MAKE_SQLSTATE(pg_diag_sqlstate[0],
+ pg_diag_sqlstate[1],
+ pg_diag_sqlstate[2],
+ pg_diag_sqlstate[3],
+ pg_diag_sqlstate[4]);
+ else
+ sqlstate = ERRCODE_CONNECTION_FAILURE;
+
+ xpstrdup(message_primary, pg_diag_message_primary);
+ xpstrdup(message_detail, pg_diag_message_detail);
+ xpstrdup(message_hint, pg_diag_message_hint);
+ xpstrdup(message_context, pg_diag_context);
+
+ if (res)
+ PQclear(res);
+
+ if (conname)
+ dblink_context_conname = conname;
+
+ ereport(level,
+ (errcode(sqlstate),
+ message_primary ? errmsg("%s", message_primary) : errmsg("unknown error"),
+ message_detail ? errdetail("%s", message_detail) : 0,
+ message_hint ? errhint("%s", message_hint) : 0,
+ message_context ? errcontext("%s", message_context) : 0,
+ errcontext("Error occurred on dblink connection named \"%s\": %s.",
+ dblink_context_conname, dblink_context_msg)));
+ }
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.23
diff -c -r1.23 dblink.out
*** expected/dblink.out 6 Apr 2008 16:54:48 -0000 1.23
--- expected/dblink.out 1 Jun 2008 21:59:11 -0000
***************
*** 125,133 ****
-- open a cursor with bad SQL and fail_on_error set to false
SELECT dblink_open('rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE: sql error
! DETAIL: ERROR: relation "foobar" does not exist
!
dblink_open
-------------
ERROR
--- 125,132 ----
-- open a cursor with bad SQL and fail_on_error set to false
SELECT dblink_open('rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE: relation "foobar" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not open cursor.
dblink_open
-------------
ERROR
***************
*** 194,202 ****
-- intentionally botch a fetch
SELECT *
FROM dblink_fetch('rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: sql error
! DETAIL: ERROR: cursor "rmt_foobar_cursor" does not exist
!
a | b | c
---+---+---
(0 rows)
--- 193,200 ----
-- intentionally botch a fetch
SELECT *
FROM dblink_fetch('rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: cursor "rmt_foobar_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor.
a | b | c
---+---+---
(0 rows)
***************
*** 210,218 ****
-- close the wrong cursor
SELECT dblink_close('rmt_foobar_cursor',false);
! NOTICE: sql error
! DETAIL: ERROR: cursor "rmt_foobar_cursor" does not exist
!
dblink_close
--------------
ERROR
--- 208,215 ----
-- close the wrong cursor
SELECT dblink_close('rmt_foobar_cursor',false);
! NOTICE: cursor "rmt_foobar_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not close cursor.
dblink_close
--------------
ERROR
***************
*** 221,235 ****
-- should generate 'cursor "rmt_foo_cursor" not found' error
SELECT *
FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR: sql error
! DETAIL: ERROR: cursor "rmt_foo_cursor" does not exist
!
-- this time, 'cursor "rmt_foo_cursor" not found' as a notice
SELECT *
FROM dblink_fetch('rmt_foo_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: sql error
! DETAIL: ERROR: cursor "rmt_foo_cursor" does not exist
!
a | b | c
---+---+---
(0 rows)
--- 218,230 ----
-- should generate 'cursor "rmt_foo_cursor" not found' error
SELECT *
FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR: cursor "rmt_foo_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor.
-- this time, 'cursor "rmt_foo_cursor" not found' as a notice
SELECT *
FROM dblink_fetch('rmt_foo_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: cursor "rmt_foo_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor.
a | b | c
---+---+---
(0 rows)
***************
*** 291,299 ****
-- bad remote select
SELECT *
FROM dblink('SELECT * FROM foobar',false) AS t(a int, b text, c text[]);
! NOTICE: sql error
! DETAIL: ERROR: relation "foobar" does not exist
!
a | b | c
---+---+---
(0 rows)
--- 286,293 ----
-- bad remote select
SELECT *
FROM dblink('SELECT * FROM foobar',false) AS t(a int, b text, c text[]);
! NOTICE: relation "foobar" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not execute query.
a | b | c
---+---+---
(0 rows)
***************
*** 316,324 ****
-- botch a change to some other data
SELECT dblink_exec('UPDATE foobar SET f3[2] = ''b99'' WHERE f1 = 11',false);
! NOTICE: sql error
! DETAIL: ERROR: relation "foobar" does not exist
!
dblink_exec
-------------
ERROR
--- 310,317 ----
-- botch a change to some other data
SELECT dblink_exec('UPDATE foobar SET f3[2] = ''b99'' WHERE f1 = 11',false);
! NOTICE: relation "foobar" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not execute command.
dblink_exec
-------------
ERROR
***************
*** 378,386 ****
SELECT *
FROM dblink('myconn','SELECT * FROM foobar',false) AS t(a int, b text, c text[])
WHERE t.a > 7;
! NOTICE: sql error
! DETAIL: ERROR: relation "foobar" does not exist
!
a | b | c
---+---+---
(0 rows)
--- 371,378 ----
SELECT *
FROM dblink('myconn','SELECT * FROM foobar',false) AS t(a int, b text, c text[])
WHERE t.a > 7;
! NOTICE: relation "foobar" does not exist
! CONTEXT: Error occurred on dblink connection named "unnamed": could not execute query.
a | b | c
---+---+---
(0 rows)
***************
*** 416,424 ****
-- open a cursor incorrectly
SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE: sql error
! DETAIL: ERROR: relation "foobar" does not exist
!
dblink_open
-------------
ERROR
--- 408,415 ----
-- open a cursor incorrectly
SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE: relation "foobar" does not exist
! CONTEXT: Error occurred on dblink connection named "myconn": could not open cursor.
dblink_open
-------------
ERROR
***************
*** 503,511 ****
-- this should fail because there is no open transaction
SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
! ERROR: sql error
! DETAIL: ERROR: DECLARE CURSOR can only be used in transaction blocks
!
-- reset remote transaction state
SELECT dblink_exec('myconn','ABORT');
dblink_exec
--- 494,501 ----
-- this should fail because there is no open transaction
SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
! ERROR: DECLARE CURSOR can only be used in transaction blocks
! CONTEXT: Error occurred on dblink connection named "unnamed": could not execute command.
-- reset remote transaction state
SELECT dblink_exec('myconn','ABORT');
dblink_exec
***************
*** 554,562 ****
-- fetch some data incorrectly
SELECT *
FROM dblink_fetch('myconn','rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: sql error
! DETAIL: ERROR: cursor "rmt_foobar_cursor" does not exist
!
a | b | c
---+---+---
(0 rows)
--- 544,551 ----
-- fetch some data incorrectly
SELECT *
FROM dblink_fetch('myconn','rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE: cursor "rmt_foobar_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "myconn": could not fetch from cursor.
a | b | c
---+---+---
(0 rows)
***************
*** 571,579 ****
-- should generate 'cursor "rmt_foo_cursor" not found' error
SELECT *
FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR: sql error
! DETAIL: ERROR: cursor "rmt_foo_cursor" does not exist
!
-- close the named persistent connection
SELECT dblink_disconnect('myconn');
dblink_disconnect
--- 560,567 ----
-- should generate 'cursor "rmt_foo_cursor" not found' error
SELECT *
FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR: cursor "rmt_foo_cursor" does not exist
! CONTEXT: Error occurred on dblink connection named "myconn": could not fetch from cursor.
-- close the named persistent connection
SELECT dblink_disconnect('myconn');
dblink_disconnect
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
