Question: RdbiPgSQL error reporting
0
gravatar for Mitchell Skinner
13.9 years ago by
Mitchell Skinner40 wrote:
I've often wished that the "Query did not produce any tuples" error message were more descriptive, so I changed it to return the postgres error message. One-line patch attached. Regards, Mitch
• 470 views
ADD COMMENTlink modified 13.9 years ago • written 13.9 years ago by Mitchell Skinner40
Answer: RdbiPgSQL error reporting
0
gravatar for Mitchell Skinner
13.9 years ago by
Mitchell Skinner40 wrote:
On Thu, 2006-01-05 at 17:00 -0800, Mitch Skinner wrote: > One-line patch attached. Hmmm, the patch attachment seems to have gotten stripped somewhere along the way--it goes like this: Index: PgSQL.c =================================================================== --- PgSQL.c (revision 15587) +++ PgSQL.c (working copy) @@ -229,7 +229,7 @@ matrix = (int)LOGICAL(asMatrix)[0]; if (PQresultStatus(result) != PGRES_TUPLES_OK) - error("Query did not produce any tuples"); + error(PQresultErrorMessage(result)); if (PQbinaryTuples(result)) error("Binary tuples not supported");
ADD COMMENTlink written 13.9 years ago by Mitchell Skinner40
Hi Mitch, Thanks for the patch! It works nice when the server actually does return an error, like in: > res = dbSendQuery(con, 'SELECT FROM mytable') > dbGetResult(res) Error in dbGetResult.PgSQL.result(res) : ERROR: syntax error at or near "FROM" at character 8 But you can still have PQresultStatus(result) != PGRES_TUPLES_OK without having the sever actually return any error. This is what happens when the query doesn't return any tuple, like in: > 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. Also, the PgSQLcolumnInfo() C function (internal for dbColumnInfo() R function) starts with: if (PQresultStatus(result) != PGRES_TUPLES_OK) error(PQresultErrorMessage(result)); Wouldn't it make sense to make the same change here too? Just wondering... I'm not really familiar with the RdbiPgSQL package though. The patch has been applied and RdbiPgSQL 1.5.1 should show up on the Bioconductor repository next Saturday morning. Regards, H. Mitch Skinner wrote: > On Thu, 2006-01-05 at 17:00 -0800, Mitch Skinner wrote: > >> One-line patch attached. > > > Hmmm, the patch attachment seems to have gotten stripped somewhere along > the way--it goes like this: > > Index: PgSQL.c > =================================================================== > --- PgSQL.c (revision 15587) > +++ PgSQL.c (working copy) > @@ -229,7 +229,7 @@ > matrix = (int)LOGICAL(asMatrix)[0]; > > if (PQresultStatus(result) != PGRES_TUPLES_OK) > - error("Query did not produce any tuples"); > + error(PQresultErrorMessage(result)); > > if (PQbinaryTuples(result)) error("Binary tuples not supported"); > > _______________________________________________ > Bioconductor mailing list > Bioconductor at stat.math.ethz.ch > https://stat.ethz.ch/mailman/listinfo/bioconductor > -- ------------------------ Hervé Pagès E-mail: hpages at fhcrc.org Phone: (206) 667-5791 Fax: (206) 667-1319
ADD REPLYlink written 13.9 years ago by Hervé Pagès ♦♦ 14k
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));
ADD REPLYlink written 13.9 years ago by Mitchell Skinner40
Please log in to add an answer.

Help
Access

Use of this site constitutes acceptance of our User Agreement and Privacy Policy.
Powered by Biostar version 16.09
Traffic: 144 users visited in the last hour