From 7177f815a6c19a8cbe8246c99f39a589d4e64b46 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 19 Mar 2013 16:07:47 +0000 Subject: [PATCH] Store a PGresult in the Exception error This makes the Diagnostics independent from further operations on the cursor and allows using it with exceptions not generated by a cursor. --- psycopg/cursor_type.c | 24 +++++++++---------- psycopg/diagnostics.h | 4 +++- psycopg/diagnostics_type.c | 38 +++++++++++++---------------- psycopg/error.h | 5 +++- psycopg/error_type.c | 34 ++++++++++++++++++-------- psycopg/lobject.h | 4 ++-- psycopg/lobject_type.c | 2 +- psycopg/microprotocols.c | 2 +- psycopg/pqpath.c | 49 ++++++++++++++++++++++++++++---------- psycopg/psycopg.h | 3 +-- psycopg/psycopgmodule.c | 26 +++++++------------- psycopg/utils.c | 6 +++++ 12 files changed, 115 insertions(+), 82 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 9570f914..a6b8c80b 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -118,7 +118,7 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) if (kind == 2) { Py_XDECREF(n); psyco_set_error(ProgrammingError, curs, - "argument formats can't be mixed", NULL, NULL); + "argument formats can't be mixed"); return -1; } kind = 1; @@ -190,7 +190,7 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) /* we found %( but not a ) */ Py_XDECREF(n); psyco_set_error(ProgrammingError, curs, - "incomplete placeholder: '%(' without ')'", NULL, NULL); + "incomplete placeholder: '%(' without ')'"); return -1; } c = d + 1; /* after the ) */ @@ -205,7 +205,7 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) if (kind == 1) { Py_XDECREF(n); psyco_set_error(ProgrammingError, curs, - "argument formats can't be mixed", NULL, NULL); + "argument formats can't be mixed"); return -1; } kind = 2; @@ -267,7 +267,7 @@ static PyObject *_psyco_curs_validate_sql_basic( if (!sql || !PyObject_IsTrue(sql)) { psyco_set_error(ProgrammingError, self, - "can't execute an empty query", NULL, NULL); + "can't execute an empty query"); goto fail; } @@ -338,8 +338,7 @@ _psyco_curs_merge_query_args(cursorObject *self, if (!strcmp(s, "not enough arguments for format string") || !strcmp(s, "not all arguments converted")) { Dprintf("psyco_curs_execute: -> got a match"); - psyco_set_error(ProgrammingError, self, - s, NULL, NULL); + psyco_set_error(ProgrammingError, self, s); pe = 1; } @@ -482,13 +481,12 @@ psyco_curs_execute(cursorObject *self, PyObject *args, PyObject *kwargs) if (self->name != NULL) { if (self->query != Py_None) { psyco_set_error(ProgrammingError, self, - "can't call .execute() on named cursors more than once", - NULL, NULL); + "can't call .execute() on named cursors more than once"); return NULL; } if (self->conn->autocommit) { psyco_set_error(ProgrammingError, self, - "can't use a named cursor outside of transactions", NULL, NULL); + "can't use a named cursor outside of transactions"); return NULL; } EXC_IF_NO_MARK(self); @@ -533,7 +531,7 @@ psyco_curs_executemany(cursorObject *self, PyObject *args, PyObject *kwargs) if (self->name != NULL) { psyco_set_error(ProgrammingError, self, - "can't call .executemany() on named cursors", NULL, NULL); + "can't call .executemany() on named cursors"); return NULL; } @@ -1038,7 +1036,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (self->name != NULL) { psyco_set_error(ProgrammingError, self, - "can't call .callproc() on named cursors", NULL, NULL); + "can't call .callproc() on named cursors"); goto exit; } @@ -1165,13 +1163,13 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) newpos = value; } else { psyco_set_error(ProgrammingError, self, - "scroll mode must be 'relative' or 'absolute'", NULL, NULL); + "scroll mode must be 'relative' or 'absolute'"); return NULL; } if (newpos < 0 || newpos >= self->rowcount ) { psyco_set_error(ProgrammingError, self, - "scroll destination out of bounds", NULL, NULL); + "scroll destination out of bounds"); return NULL; } diff --git a/psycopg/diagnostics.h b/psycopg/diagnostics.h index adaf7580..b8f716d8 100644 --- a/psycopg/diagnostics.h +++ b/psycopg/diagnostics.h @@ -26,12 +26,14 @@ #ifndef PSYCOPG_DIAGNOSTICS_H #define PSYCOPG_DIAGNOSTICS_H 1 +#include "psycopg/error.h" + extern HIDDEN PyTypeObject diagnosticsType; typedef struct { PyObject_HEAD - PyObject *err; /* exception to retrieve the diagnostics from */ + PsycoErrorObject *err; /* exception to retrieve the diagnostics from */ } diagnosticsObject; diff --git a/psycopg/diagnostics_type.c b/psycopg/diagnostics_type.c index 30ef2d46..0dd6b7a9 100644 --- a/psycopg/diagnostics_type.c +++ b/psycopg/diagnostics_type.c @@ -27,7 +27,7 @@ #include "psycopg/psycopg.h" #include "psycopg/diagnostics.h" -#include "psycopg/cursor.h" +#include "psycopg/error.h" /* These are new in PostgreSQL 9.3. Defining them here so that psycopg2 can * use them with a 9.3+ server even if compiled against pre-9.3 headers. */ @@ -55,27 +55,15 @@ static PyObject * psyco_diagnostics_get_field(diagnosticsObject *self, void *closure) { - // closure contains the field code. - PyObject *rv = NULL; - PyObject *curs = NULL; - const char* errortext; - if (!(curs = PyObject_GetAttrString(self->err, "cursor")) || - !PyObject_TypeCheck(curs, &cursorType) || - ((cursorObject *)curs)->pgres == NULL) { - goto exit; + const char *errortext; + + if (!self->err->pgres) { + Py_INCREF(Py_None); + return Py_None; } - errortext = PQresultErrorField( - ((cursorObject *)curs)->pgres, (Py_intptr_t) closure); - if (errortext) { - rv = conn_text_from_chars(((cursorObject *)curs)->conn, errortext); - } -exit: - if (!rv) { - rv = Py_None; - Py_INCREF(rv); - } - Py_XDECREF(curs); - return rv; + + errortext = PQresultErrorField(self->err->pgres, (Py_intptr_t) closure); + return error_text_from_chars(self->err, errortext); } @@ -134,8 +122,14 @@ diagnostics_init(diagnosticsObject *self, PyObject *args, PyObject *kwds) if (!PyArg_ParseTuple(args, "O", &err)) return -1; + if (!PyObject_TypeCheck(err, &ErrorType)) { + PyErr_SetString(PyExc_TypeError, + "The argument must be a psycopg2.Error"); + return -1; + } + Py_INCREF(err); - self->err = err; + self->err = (PsycoErrorObject *)err; return 0; } diff --git a/psycopg/error.h b/psycopg/error.h index f6551d50..c5070d87 100644 --- a/psycopg/error.h +++ b/psycopg/error.h @@ -34,7 +34,10 @@ typedef struct { PyObject *pgerror; PyObject *pgcode; cursorObject *cursor; - PGresult *result; + char *codec; + PGresult *pgres; } PsycoErrorObject; +HIDDEN PyObject *error_text_from_chars(PsycoErrorObject *self, const char *str); + #endif /* PSYCOPG_ERROR_H */ diff --git a/psycopg/error_type.c b/psycopg/error_type.c index cc1a5c0c..5fc47eca 100644 --- a/psycopg/error_type.c +++ b/psycopg/error_type.c @@ -31,6 +31,23 @@ #include "psycopg/pqpath.h" +PyObject * +error_text_from_chars(PsycoErrorObject *self, const char *str) +{ + if (str == NULL) { + Py_INCREF(Py_None); + return (Py_None); + } + +#if PY_MAJOR_VERSION < 3 + return PyString_FromString(str); +#else + return PyUnicode_Decode(str, strlen(str), + self->codec ? self->codec : "ascii", "replace"); +#endif +} + + static const char pgerror_doc[] = "The error message returned by the backend, if available, else None"; @@ -40,6 +57,9 @@ static const char pgcode_doc[] = static const char cursor_doc[] = "The cursor that raised the exception, if available, else None"; +static const char diag_doc[] = + "A Diagnostics object to get further information about the error"; + static PyMemberDef error_members[] = { { "pgerror", T_OBJECT, offsetof(PsycoErrorObject, pgerror), READONLY, (char *)pgerror_doc }, @@ -64,18 +84,12 @@ error_init(PsycoErrorObject *self, PyObject *args, PyObject *kwargs) (PyObject *)self, args, kwargs) < 0) { return -1; } - Py_CLEAR(self->pgerror); - Py_CLEAR(self->pgcode); - Py_CLEAR(self->cursor); - IFCLEARPGRES(self->result); return 0; } static int error_traverse(PsycoErrorObject *self, visitproc visit, void *arg) { - Py_VISIT(self->pgerror); - Py_VISIT(self->pgcode); Py_VISIT(self->cursor); return ((PyTypeObject *)PyExc_StandardError)->tp_traverse( (PyObject *)self, visit, arg); @@ -87,7 +101,8 @@ error_clear(PsycoErrorObject *self) Py_CLEAR(self->pgerror); Py_CLEAR(self->pgcode); Py_CLEAR(self->cursor); - IFCLEARPGRES(self->result); + PyMem_Free(self->codec); + IFCLEARPGRES(self->pgres); return ((PyTypeObject *)PyExc_StandardError)->tp_clear((PyObject *)self); } @@ -107,9 +122,8 @@ error_get_diag(PsycoErrorObject *self, void *closure) } static struct PyGetSetDef error_getsets[] = { - { "diag", (getter)error_get_diag, NULL, - "Database error diagnostics" }, - {NULL} + { "diag", (getter)error_get_diag, NULL, (char *)diag_doc }, + { NULL } }; diff --git a/psycopg/lobject.h b/psycopg/lobject.h index 6587198c..56f9ead4 100644 --- a/psycopg/lobject.h +++ b/psycopg/lobject.h @@ -78,13 +78,13 @@ RAISES_NEG HIDDEN int lobject_close(lobjectObject *self); #define EXC_IF_LOBJ_LEVEL0(self) \ if (self->conn->autocommit) { \ psyco_set_error(ProgrammingError, NULL, \ - "can't use a lobject outside of transactions", NULL, NULL); \ + "can't use a lobject outside of transactions"); \ return NULL; \ } #define EXC_IF_LOBJ_UNMARKED(self) \ if (self->conn->mark != self->mark) { \ psyco_set_error(ProgrammingError, NULL, \ - "lobject isn't valid anymore", NULL, NULL); \ + "lobject isn't valid anymore"); \ return NULL; \ } diff --git a/psycopg/lobject_type.c b/psycopg/lobject_type.c index 625a2939..fb477522 100644 --- a/psycopg/lobject_type.c +++ b/psycopg/lobject_type.c @@ -333,7 +333,7 @@ lobject_setup(lobjectObject *self, connectionObject *conn, if (conn->autocommit) { psyco_set_error(ProgrammingError, NULL, - "can't use a lobject outside of transactions", NULL, NULL); + "can't use a lobject outside of transactions"); return -1; } diff --git a/psycopg/microprotocols.c b/psycopg/microprotocols.c index 23f12794..1687bc26 100644 --- a/psycopg/microprotocols.c +++ b/psycopg/microprotocols.c @@ -203,7 +203,7 @@ microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) /* else set the right exception and return NULL */ PyOS_snprintf(buffer, 255, "can't adapt type '%s'", Py_TYPE(obj)->tp_name); - psyco_set_error(ProgrammingError, NULL, buffer, NULL, NULL); + psyco_set_error(ProgrammingError, NULL, buffer); return NULL; } diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 62b8c157..107d58ce 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -38,6 +38,7 @@ #include "psycopg/green.h" #include "psycopg/typecast.h" #include "psycopg/pgtypes.h" +#include "psycopg/error.h" #include @@ -149,15 +150,20 @@ exception_from_sqlstate(const char *sqlstate) /* pq_raise - raise a python exception of the right kind - This function should be called while holding the GIL. */ + This function should be called while holding the GIL. + + The function passes the ownership of the pgres to the returned exception, + wherer the pgres was the explicit argument or taken from the cursor. + So, after calling it curs->pgres will be set to null */ RAISES static void -pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) +pq_raise(connectionObject *conn, cursorObject *curs, PGresult **pgres) { PyObject *exc = NULL; const char *err = NULL; const char *err2 = NULL; const char *code = NULL; + PyObject *pyerr = NULL; if (conn == NULL) { PyErr_SetString(DatabaseError, @@ -171,13 +177,13 @@ pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) conn->closed = 2; if (pgres == NULL && curs != NULL) - pgres = curs->pgres; + pgres = &curs->pgres; - if (pgres) { - err = PQresultErrorMessage(pgres); + if (pgres && *pgres) { + err = PQresultErrorMessage(*pgres); if (err != NULL) { Dprintf("pq_raise: PQresultErrorMessage: err=%s", err); - code = PQresultErrorField(pgres, PG_DIAG_SQLSTATE); + code = PQresultErrorField(*pgres, PG_DIAG_SQLSTATE); } } if (err == NULL) { @@ -210,7 +216,26 @@ pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) err2 = strip_severity(err); Dprintf("pq_raise: err2=%s", err2); - psyco_set_error(exc, curs, err2, err, code); + pyerr = psyco_set_error(exc, curs, err2); + + if (pyerr && PyObject_TypeCheck(pyerr, &ErrorType)) { + PsycoErrorObject *perr = (PsycoErrorObject *)pyerr; + + PyMem_Free(perr->codec); + psycopg_strdup(&perr->codec, conn->codec, 0); + + Py_CLEAR(perr->pgerror); + perr->pgerror = error_text_from_chars(perr, err); + + Py_CLEAR(perr->pgcode); + perr->pgcode = error_text_from_chars(perr, code); + + IFCLEARPGRES(perr->pgres); + if (pgres && *pgres) { + perr->pgres = *pgres; + *pgres = NULL; + } + } } /* pq_set_critical, pq_resolve_critical - manage critical errors @@ -388,14 +413,16 @@ pq_complete_error(connectionObject *conn, PGresult **pgres, char **error) { Dprintf("pq_complete_error: pgconn = %p, pgres = %p, error = %s", conn->pgconn, *pgres, *error ? *error : "(null)"); - if (*pgres != NULL) - pq_raise(conn, NULL, *pgres); + if (*pgres != NULL) { + pq_raise(conn, NULL, pgres); + /* now *pgres is null */ + } else if (*error != NULL) { PyErr_SetString(OperationalError, *error); } else { PyErr_SetString(OperationalError, "unknown error"); } - IFCLEARPGRES(*pgres); + if (*error) { free(*error); *error = NULL; @@ -1509,8 +1536,6 @@ pq_fetch(cursorObject *curs, int no_result) default: Dprintf("pq_fetch: uh-oh, something FAILED: pgconn = %p", curs->conn); pq_raise(curs->conn, curs, NULL); - /* don't clear curs->pgres, because it contains detailed error - information */ ex = -1; break; } diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 2e86dca8..cc8281ed 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -120,8 +120,7 @@ HIDDEN PyObject *psyco_GetDecimalType(void); typedef struct cursorObject cursorObject; /* some utility functions */ -RAISES HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, - const char *pgerror, const char *pgcode); +RAISES HIDDEN PyObject *psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg); HIDDEN char *psycopg_escape_string(PyObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 1b4b0d6f..7255bfa2 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -512,11 +512,10 @@ psyco_errors_set(PyObject *type) Create a new error of the given type with extra attributes. */ -RAISES void -psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, - const char *pgerror, const char *pgcode) +/* TODO: may have been changed to BORROWED */ +RAISES PyObject * +psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg) { - PyObject *t; PyObject *pymsg; PyObject *err = NULL; connectionObject *conn = NULL; @@ -532,31 +531,24 @@ psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, else { /* what's better than an error in an error handler in the morning? * Anyway, some error was set, refcount is ok... get outta here. */ - return; + return NULL; } if (err && PyObject_TypeCheck(err, &ErrorType)) { PsycoErrorObject *perr = (PsycoErrorObject *)err; if (curs) { + Py_CLEAR(perr->cursor); Py_INCREF(curs); perr->cursor = curs; } + } - if (pgerror) { - if ((t = conn_text_from_chars(conn, pgerror))) { - perr->pgerror = t; - } - } - - if (pgcode) { - if ((t = conn_text_from_chars(conn, pgcode))) { - perr->pgcode = t; - } - } - + if (err) { PyErr_SetObject(exc, err); Py_DECREF(err); } + + return err; } diff --git a/psycopg/utils.c b/psycopg/utils.c index 57586c57..698dcb49 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -115,10 +115,16 @@ psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) * * Store the return in 'to' and return 0 in case of success, else return -1 * and raise an exception. + * + * If from is null, store null into to. */ RAISES_NEG int psycopg_strdup(char **to, const char *from, Py_ssize_t len) { + if (!from) { + *to = NULL; + return 0; + } if (!len) { len = strlen(from); } if (!(*to = PyMem_Malloc(len + 1))) { PyErr_NoMemory();