[BioC] RdbiPgSQL error reporting

[Mitch Skinner]

Thanks for the response.  It's always nice to get detailed feedback on
something.  I've added checks for the things you mentioned:

On Thu, 2006-01-05 at 19:38 -0800, Herve Pages wrote:
>  > res = dbSendQuery(con, "INSERT INTO mytable VALUES ('bla', 'bla')")
> Then with the patched PgSQLgetResult() function, you get:
>  > dbGetResult(res)
> Error in dbGetResult.PgSQL.result(res) :
> 
> No error message at all!
> I agree this is the consequence of the user misusing the dbGetResult() 
> function.
> But it would be nice if we could keep the "Query did not produce any tuples"
> message for this special case. 

We're in luck; there are two separate PQresultStatus return values to
distinguish those cases, one for successful no-tuple-producing commands
(like your insert example), and one for successful tuple-producing
commands (queries).  I've added a new check in the diff below.

> Also, the PgSQLcolumnInfo() C function
> (internal for dbColumnInfo() R function) starts with:
> if (PQresultStatus(result) != PGRES_TUPLES_OK)
> 
> Wouldn't it make sense to make the same change here too?
> Just wondering... I'm not really familiar with the RdbiPgSQL
> package though.

I agree, this makes sense.  The other case is the combination of the two
things you mentioned (trying to do a dbColumnInfo on the result of
something like an INSERT).

With the extra checks:
> res <- dbSendQuery(conn, "INSERT INTO mytable VALUES ('bla', 'bla')")
> dbGetResult(res)
Error in dbGetResult.PgSQL.result(res) : Query did not produce any
tuples
> dbColumnInfo(res)
Error in dbColumnInfo.PgSQL.result(res) : Query did not produce any
tuples
> res <- dbSendQuery(conn, "SELECT FROM mytable")
> dbGetResult(res)
Error in dbGetResult.PgSQL.result(res) : ERROR:  syntax error at or near
"FROM" at character 8
> dbColumnInfo(res)
Error in dbColumnInfo.PgSQL.result(res) : ERROR:  syntax error at or
near "FROM" at character 8

The diff below is against current SVN (meaning, this diff assumes that
my change from yesterday is already there).

Style-wise, maybe it ought to use a switch statement, or maybe now that
there are four lines of identical code in two functions they should get
factored into their own function.  I don't think it matters much in this
case--this message includes what I think is the minimal change, without
any larger-scale refactoring.  If anyone has a strong opinion about it
(or any other comments), let me know and I'll re-do.

Regards,
Mitch

Index: PgSQL.c
===================================================================
--- PgSQL.c     (revision 15600)
+++ PgSQL.c     (working copy)
@@ -181,9 +181,12 @@
   SEXP outFrame, rowNames, colNames;
   PGresult *result = (PGresult *) R_ExternalPtrAddr(resultPtr);

-  if (PQresultStatus(result) != PGRES_TUPLES_OK)
+  if (PQresultStatus(result) == PGRES_COMMAND_OK)
     error("Query did not produce any tuples");

+  if (PQresultStatus(result) != PGRES_TUPLES_OK)
+    error(PQresultErrorMessage(result));
+
   rows = PQnfields(result);

   PROTECT(outFrame = allocVector(VECSXP, 3));
@@ -228,6 +231,9 @@

   matrix = (int)LOGICAL(asMatrix)[0];

+  if (PQresultStatus(result) == PGRES_COMMAND_OK)
+    error("Query did not produce any tuples");
+
   if (PQresultStatus(result) != PGRES_TUPLES_OK)
     error(PQresultErrorMessage(result));