Don't use default_transaction_* for session characteristics

Store the state in the connection object and set the params on BEGIN

Some tests fail: a few can be fixed reading transaction_* instead of
default_transaction_*; but the behaviour of tx characteristics with
autocommit is effectively changed. It may be addressed by setting
default_transaction_* if autocommit is set.

psycopg/connection.h

@@ -38,6 +38,12 @@ extern "C" {
 #define ISOLATION_LEVEL_READ_COMMITTED      1
 #define ISOLATION_LEVEL_REPEATABLE_READ     2
 #define ISOLATION_LEVEL_SERIALIZABLE        3
+#define ISOLATION_LEVEL_DEFAULT             5
+
+/* 3-state values on/off/default */
+#define STATE_OFF               0
+#define STATE_ON                1
+#define STATE_DEFAULT           2
 
 /* connection status */
 #define CONN_STATUS_SETUP       0
@@ -129,6 +135,11 @@ struct connectionObject {
      * codecs.getdecoder('utf8') */
     PyObject *pyencoder;        /* python codec encoding function */
     PyObject *pydecoder;        /* python codec decoding function */
+
+    /* Values for the transactions characteristics */
+    int isolevel;
+    int readonly;
+    int deferrable;
 };
 
 /* map isolation level values into a numeric const */
@@ -155,10 +166,9 @@ HIDDEN void conn_close(connectionObject *self);
 HIDDEN void conn_close_locked(connectionObject *self);
 RAISES_NEG HIDDEN int  conn_commit(connectionObject *self);
 RAISES_NEG HIDDEN int  conn_rollback(connectionObject *self);
-RAISES_NEG HIDDEN int  conn_set_session(connectionObject *self, const char *isolevel,
-                                 const char *readonly, const char *deferrable,
-                                 int autocommit);
 HIDDEN int  conn_set_autocommit(connectionObject *self, int value);
+RAISES_NEG HIDDEN int  conn_parse_isolevel(connectionObject *self, PyObject *pyval);
+RAISES_NEG HIDDEN int  conn_parse_onoff(PyObject *pyval);
 RAISES_NEG HIDDEN int  conn_switch_isolation_level(connectionObject *self, int level);
 RAISES_NEG HIDDEN int  conn_set_client_encoding(connectionObject *self, const char *enc);
 HIDDEN int  conn_poll(connectionObject *self);

psycopg/connection_int.c

@@ -34,20 +34,29 @@
 
 #include <string.h>
 
-/* Mapping from isolation level name to value exposed by Python.
+/* String indexes match the ISOLATION_LEVEL_* consts */
- *
+const char *srv_isolevels[] = {
- * Note: ordering matters: to get a valid pre-PG 8 level from one not valid,
+    NULL, /* autocommit */
- * we increase a pointer in this list by one position. */
+    "READ COMMITTED",
-const IsolationLevel conn_isolevels[] = {
+    "REPEATABLE READ",
-    {"",                    ISOLATION_LEVEL_AUTOCOMMIT},
+    "SERIALIZABLE",
-    {"read uncommitted",    ISOLATION_LEVEL_READ_UNCOMMITTED},
+    "READ UNCOMMITTED",
-    {"read committed",      ISOLATION_LEVEL_READ_COMMITTED},
+    ""      /* default */
-    {"repeatable read",     ISOLATION_LEVEL_REPEATABLE_READ},
-    {"serializable",        ISOLATION_LEVEL_SERIALIZABLE},
-    {"default",            -1},     /* never to be found on the server */
-    { NULL }
 };
 
+/* Read only false, true */
+const char *srv_readonly[] = {
+    " READ WRITE",
+    " READ ONLY",
+    ""      /* default */
+};
+
+/* Deferrable false, true */
+const char *srv_deferrable[] = {
+    " NOT DEFERRABLE",
+    " DEFERRABLE",
+    ""      /* default */
+};
 
 /* Return a new "string" from a char* from the database.
  *
@@ -553,50 +562,13 @@ exit:
 RAISES_NEG int
 conn_get_isolation_level(connectionObject *self)
 {
-    PGresult *pgres = NULL;
-    char *error = NULL;
-    int rv = -1;
-    char *lname;
-    const IsolationLevel *level;
-
     /* this may get called by async connections too: here's your result */
     if (self->autocommit) {
-        return 0;
+        return ISOLATION_LEVEL_AUTOCOMMIT;
     }
-
+    else {
-    Py_BEGIN_ALLOW_THREADS;
+        return self->isolevel;
-    pthread_mutex_lock(&self->lock);
-
-    if (!(lname = pq_get_guc_locked(self, "default_transaction_isolation",
-            &pgres, &error, &_save))) {
-        goto endlock;
     }
-
-    /* find the value for the requested isolation level */
-    level = conn_isolevels;
-    while ((++level)->name) {
-        if (0 == strcasecmp(level->name, lname)) {
-            rv = level->value;
-            break;
-        }
-    }
-    if (-1 == rv) {
-        error = malloc(256);
-        PyOS_snprintf(error, 256,
-            "unexpected isolation level: '%s'", lname);
-    }
-
-    free(lname);
-
-endlock:
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;
-
-    if (rv < 0) {
-        pq_complete_error(self, &pgres, &error);
-    }
-
-    return rv;
 }
 
 
@@ -1208,63 +1180,6 @@ conn_rollback(connectionObject *self)
     return res;
 }
 
-RAISES_NEG int
-conn_set_session(connectionObject *self,
-        const char *isolevel, const char *readonly, const char *deferrable,
-        int autocommit)
-{
-    PGresult *pgres = NULL;
-    char *error = NULL;
-    int res = -1;
-
-    Py_BEGIN_ALLOW_THREADS;
-    pthread_mutex_lock(&self->lock);
-
-    if (isolevel) {
-        Dprintf("conn_set_session: setting isolation to %s", isolevel);
-        if ((res = pq_set_guc_locked(self,
-                "default_transaction_isolation", isolevel,
-                &pgres, &error, &_save))) {
-            goto endlock;
-        }
-    }
-
-    if (readonly) {
-        Dprintf("conn_set_session: setting read only to %s", readonly);
-        if ((res = pq_set_guc_locked(self,
-                "default_transaction_read_only", readonly,
-                &pgres, &error, &_save))) {
-            goto endlock;
-        }
-    }
-
-    if (deferrable) {
-        Dprintf("conn_set_session: setting deferrable to %s", deferrable);
-        if ((res = pq_set_guc_locked(self,
-                "default_transaction_deferrable", deferrable,
-                &pgres, &error, &_save))) {
-            goto endlock;
-        }
-    }
-
-    if (self->autocommit != autocommit) {
-        Dprintf("conn_set_session: setting autocommit to %d", autocommit);
-        self->autocommit = autocommit;
-    }
-
-    res = 0;
-
-endlock:
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;
-
-    if (res < 0) {
-        pq_complete_error(self, &pgres, &error);
-    }
-
-    return res;
-}
-
 int
 conn_set_autocommit(connectionObject *self, int value)
 {
@@ -1279,85 +1194,126 @@ conn_set_autocommit(connectionObject *self, int value)
     return 0;
 }
 
+/* Promote an isolation level to one of the levels supported by the server */
+
+static int _adjust_isolevel(connectionObject *self, int level) {
+    if (self->server_version < 80000) {
+        if (level == ISOLATION_LEVEL_READ_UNCOMMITTED) {
+            level = ISOLATION_LEVEL_READ_COMMITTED;
+        }
+        else if (level == ISOLATION_LEVEL_REPEATABLE_READ) {
+            level = ISOLATION_LEVEL_SERIALIZABLE;
+        }
+    }
+    return level;
+}
+
+
+/* parse a python object into one of the possible isolation level values */
+
+RAISES_NEG int
+conn_parse_isolevel(connectionObject *self, PyObject *pyval)
+{
+    int rv = -1;
+    long level;
+
+    Py_INCREF(pyval);   /* for ensure_bytes */
+
+    /* parse from one of the level constants */
+    if (PyInt_Check(pyval)) {
+        level = PyInt_AsLong(pyval);
+        if (level == -1 && PyErr_Occurred()) { goto exit; }
+        if (level < 1 || level > 4) {
+            PyErr_SetString(PyExc_ValueError,
+                "isolation_level must be between 1 and 4");
+            goto exit;
+        }
+
+        rv = level;
+    }
+
+    /* parse from the string -- this includes "default" */
+
+    else {
+        if (!(pyval = psycopg_ensure_bytes(pyval))) {
+            goto exit;
+        }
+        for (level = 1; level <= 4; level++) {
+            if (0 == strcasecmp(srv_isolevels[level], Bytes_AS_STRING(pyval))) {
+                rv = level;
+                break;
+            }
+        }
+        if (rv < 0 && 0 == strcasecmp("default", Bytes_AS_STRING(pyval))) {
+            rv = ISOLATION_LEVEL_DEFAULT;
+        }
+        if (rv < 0) {
+            PyErr_Format(PyExc_ValueError,
+                "bad value for isolation_level: '%s'", Bytes_AS_STRING(pyval));
+            goto exit;
+        }
+    }
+
+    rv = _adjust_isolevel(self, rv);
+
+exit:
+    Py_XDECREF(pyval);
+
+    return rv;
+}
+
+/* convert False/True/"default" -> 0/1/2 */
+
+RAISES_NEG int
+conn_parse_onoff(PyObject *pyval)
+{
+    int rv = -1;
+
+    Py_INCREF(pyval);   /* for ensure_bytes */
+
+    if (PyUnicode_CheckExact(pyval) || Bytes_CheckExact(pyval)) {
+        if (!(pyval = psycopg_ensure_bytes(pyval))) {
+            goto exit;
+        }
+        if (0 == strcasecmp("default", Bytes_AS_STRING(pyval))) {
+            rv = STATE_DEFAULT;
+        }
+        else {
+            PyErr_Format(PyExc_ValueError,
+                "the only string accepted is 'default'; got %s",
+                Bytes_AS_STRING(pyval));
+            goto exit;
+        }
+    }
+    else {
+        int istrue;
+        if (0 > (istrue = PyObject_IsTrue(pyval))) { goto exit; }
+        rv = istrue ? STATE_ON : STATE_OFF;
+    }
+
+exit:
+    Py_XDECREF(pyval);
+
+    return rv;
+}
+
 /* conn_switch_isolation_level - switch isolation level on the connection */
 
 RAISES_NEG int
 conn_switch_isolation_level(connectionObject *self, int level)
 {
-    PGresult *pgres = NULL;
-    char *error = NULL;
-    int curr_level;
-    int ret = -1;
-
-    /* use only supported levels on older PG versions */
-    if (self->server_version < 80000) {
-        if (level == ISOLATION_LEVEL_READ_UNCOMMITTED)
-            level = ISOLATION_LEVEL_READ_COMMITTED;
-        else if (level == ISOLATION_LEVEL_REPEATABLE_READ)
-            level = ISOLATION_LEVEL_SERIALIZABLE;
-    }
-
-    if (-1 == (curr_level = conn_get_isolation_level(self))) {
-        return -1;
-    }
-
-    if (curr_level == level) {
-        /* no need to change level */
-        return 0;
-    }
-
-    /* Emulate the previous semantic of set_isolation_level() using the
-     * functions currently available. */
-
-    Py_BEGIN_ALLOW_THREADS;
-    pthread_mutex_lock(&self->lock);
-
-    /* terminate the current transaction if any */
-    if ((ret = pq_abort_locked(self, &pgres, &error, &_save))) {
-        goto endlock;
-    }
-
     if (level == 0) {
-        if ((ret = pq_set_guc_locked(self,
-                "default_transaction_isolation", "default",
-                &pgres, &error, &_save))) {
-            goto endlock;
-        }
         self->autocommit = 1;
     }
     else {
-        /* find the name of the requested level */
+        level = _adjust_isolevel(self, level);
-        const IsolationLevel *isolevel = conn_isolevels;
+        self->isolevel = level;
-        while ((++isolevel)->name) {
-            if (level == isolevel->value) {
-                break;
-            }
-        }
-        if (!isolevel->name) {
-            ret = -1;
-            error = strdup("bad isolation level value");
-            goto endlock;
-        }
-
-        if ((ret = pq_set_guc_locked(self,
-                "default_transaction_isolation", isolevel->name,
-                &pgres, &error, &_save))) {
-            goto endlock;
-        }
         self->autocommit = 0;
     }
 
     Dprintf("conn_switch_isolation_level: switched to level %d", level);
 
-endlock:
+    return 0;
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;
-
-    if (ret < 0) {
-        pq_complete_error(self, &pgres, &error);
-    }
-
-    return ret;
 }
 
 

psycopg/connection_type.c

@@ -442,86 +442,6 @@ exit:
 }
 
 
-/* parse a python object into one of the possible isolation level values */
-
-extern const IsolationLevel conn_isolevels[];
-
-static const char *
-_psyco_conn_parse_isolevel(connectionObject *self, PyObject *pyval)
-{
-    const IsolationLevel *isolevel = NULL;
-
-    Py_INCREF(pyval);   /* for ensure_bytes */
-
-    /* parse from one of the level constants */
-    if (PyInt_Check(pyval)) {
-        long level = PyInt_AsLong(pyval);
-        if (level == -1 && PyErr_Occurred()) { goto exit; }
-        if (level < 1 || level > 4) {
-            PyErr_SetString(PyExc_ValueError,
-                "isolation_level must be between 1 and 4");
-            goto exit;
-        }
-
-        isolevel = conn_isolevels;
-        while ((++isolevel)->value != level)
-            ; /* continue */
-    }
-
-    /* parse from the string -- this includes "default" */
-    else {
-        isolevel = conn_isolevels;
-        while ((++isolevel)->name) {
-            if (!(pyval = psycopg_ensure_bytes(pyval))) {
-                goto exit;
-            }
-            if (0 == strcasecmp(isolevel->name, Bytes_AS_STRING(pyval))) {
-                break;
-            }
-        }
-        if (!isolevel->name) {
-            char msg[256];
-            snprintf(msg, sizeof(msg),
-                "bad value for isolation_level: '%s'", Bytes_AS_STRING(pyval));
-            PyErr_SetString(PyExc_ValueError, msg);
-        }
-    }
-
-    /* use only supported levels on older PG versions */
-    if (isolevel && self->server_version < 80000) {
-        if (isolevel->value == ISOLATION_LEVEL_READ_UNCOMMITTED
-            || isolevel->value == ISOLATION_LEVEL_REPEATABLE_READ) {
-            ++isolevel;
-        }
-    }
-
-exit:
-    Py_XDECREF(pyval);
-
-    return isolevel ? isolevel->name : NULL;
-}
-
-/* convert True/False/"default" into a C string */
-
-static const char *
-_psyco_conn_parse_onoff(PyObject *pyval)
-{
-    int istrue = PyObject_IsTrue(pyval);
-    if (-1 == istrue) { return NULL; }
-    if (istrue) {
-        int cmp;
-        PyObject *pydef;
-        if (!(pydef = Text_FromUTF8("default"))) { return NULL; }
-        cmp = PyObject_RichCompareBool(pyval, pydef, Py_EQ);
-        Py_DECREF(pydef);
-        if (-1 == cmp) { return NULL; }
-        return cmp ? "default" : "on";
-    }
-    else {
-        return "off";
-    }
-}
-
 /* set_session - set default transaction characteristics */
 
 #define psyco_conn_set_session_doc \
@@ -536,9 +456,9 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs)
     PyObject *deferrable = Py_None;
     PyObject *autocommit = Py_None;
 
-    const char *c_isolevel = NULL;
+    int c_isolevel = self->isolevel;
-    const char *c_readonly = NULL;
+    int c_readonly = self->readonly;
-    const char *c_deferrable = NULL;
+    int c_deferrable = self->deferrable;
     int c_autocommit = self->autocommit;
 
     static char *kwlist[] =
@@ -554,13 +474,13 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs)
     }
 
     if (Py_None != isolevel) {
-        if (!(c_isolevel = _psyco_conn_parse_isolevel(self, isolevel))) {
+        if (0 > (c_isolevel = conn_parse_isolevel(self, isolevel))) {
             return NULL;
         }
     }
 
     if (Py_None != readonly) {
-        if (!(c_readonly = _psyco_conn_parse_onoff(readonly))) {
+        if (0 > (c_readonly = conn_parse_onoff(readonly))) {
             return NULL;
         }
     }
@@ -571,19 +491,19 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs)
                 " from PostgreSQL 9.1");
             return NULL;
         }
-        if (!(c_deferrable = _psyco_conn_parse_onoff(deferrable))) {
+        if (0 > (c_deferrable = conn_parse_onoff(readonly))) {
             return NULL;
         }
     }
+
     if (Py_None != autocommit) {
-        c_autocommit = PyObject_IsTrue(autocommit);
+        if (-1 == (c_autocommit = PyObject_IsTrue(autocommit))) { return NULL; }
-        if (-1 == c_autocommit) { return NULL; }
     }
 
-    if (0 > conn_set_session(self,
+    self->isolevel = c_isolevel;
-            c_isolevel, c_readonly, c_deferrable, c_autocommit)) {
+    self->readonly = c_readonly;
-        return NULL;
+    self->deferrable = c_deferrable;
-    }
+    self->autocommit = c_autocommit;
 
     Py_RETURN_NONE;
 }
@@ -1107,6 +1027,9 @@ connection_setup(connectionObject *self, const char *dsn, long int async)
     self->async_status = ASYNC_DONE;
     if (!(self->string_types = PyDict_New())) { goto exit; }
     if (!(self->binary_types = PyDict_New())) { goto exit; }
+    self->isolevel = ISOLATION_LEVEL_DEFAULT;
+    self->readonly = STATE_DEFAULT;
+    self->deferrable = STATE_DEFAULT;
     /* other fields have been zeroed by tp_alloc */
 
     pthread_mutex_init(&(self->lock), NULL);

psycopg/pqpath.c

@@ -53,7 +53,9 @@
 #endif
 
 extern HIDDEN PyObject *psyco_DescriptionType;
-
+extern HIDDEN const char *srv_isolevels[];
+extern HIDDEN const char *srv_readonly[];
+extern HIDDEN const char *srv_deferrable[];
 
 /* Strip off the severity from a Postgres error message. */
 static const char *
@@ -479,6 +481,8 @@ int
 pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error,
                 PyThreadState **tstate)
 {
+    const size_t bufsize = 256;
+    char buf[bufsize];
     int result;
 
     Dprintf("pq_begin_locked: pgconn = %p, autocommit = %d, status = %d",
@@ -489,7 +493,14 @@ pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error,
         return 0;
     }
 
-    result = pq_execute_command_locked(conn, "BEGIN", pgres, error, tstate);
+    snprintf(buf, bufsize, "BEGIN%s%s%s%s%s",
+        conn->server_version < 80000 ? ";SET TRANSACTION" : "",
+        (conn->isolevel >= 1 && conn->isolevel <= 4) ? " ISOLATION LEVEL " : "",
+        srv_isolevels[conn->isolevel],
+        srv_readonly[conn->readonly],
+        srv_deferrable[conn->deferrable]);
+
+    result = pq_execute_command_locked(conn, buf, pgres, error, tstate);
     if (result == 0)
         conn->status = CONN_STATUS_BEGIN;