Re: [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code - Mailing list pgsql-patches

Tom Lane wrote:
> Joe Conway <mail@joeconway.com> 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