From c1e016e597e1ee2b52e5277a09793caa9401f39d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 02:25:48 +0000 Subject: [PATCH 1/8] 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 | 16 +- psycopg/connection_int.c | 304 ++++++++++++++++---------------------- psycopg/connection_type.c | 107 ++------------ psycopg/pqpath.c | 15 +- 4 files changed, 171 insertions(+), 271 deletions(-) diff --git a/psycopg/connection.h b/psycopg/connection.h index 2e2d51de..6c39263e 100644 --- a/psycopg/connection.h +++ b/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); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index f92a658e..d8d693fe 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -34,20 +34,29 @@ #include -/* Mapping from isolation level name to value exposed by Python. - * - * Note: ordering matters: to get a valid pre-PG 8 level from one not valid, - * we increase a pointer in this list by one position. */ -const IsolationLevel conn_isolevels[] = { - {"", ISOLATION_LEVEL_AUTOCOMMIT}, - {"read uncommitted", ISOLATION_LEVEL_READ_UNCOMMITTED}, - {"read committed", ISOLATION_LEVEL_READ_COMMITTED}, - {"repeatable read", ISOLATION_LEVEL_REPEATABLE_READ}, - {"serializable", ISOLATION_LEVEL_SERIALIZABLE}, - {"default", -1}, /* never to be found on the server */ - { NULL } +/* String indexes match the ISOLATION_LEVEL_* consts */ +const char *srv_isolevels[] = { + NULL, /* autocommit */ + "READ COMMITTED", + "REPEATABLE READ", + "SERIALIZABLE", + "READ UNCOMMITTED", + "" /* default */ }; +/* 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; } - - Py_BEGIN_ALLOW_THREADS; - pthread_mutex_lock(&self->lock); - - if (!(lname = pq_get_guc_locked(self, "default_transaction_isolation", - &pgres, &error, &_save))) { - goto endlock; + else { + return self->isolevel; } - - /* 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 */ - const IsolationLevel *isolevel = conn_isolevels; - 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; - } + level = _adjust_isolevel(self, level); + self->isolevel = level; self->autocommit = 0; } Dprintf("conn_switch_isolation_level: switched to level %d", level); -endlock: - pthread_mutex_unlock(&self->lock); - Py_END_ALLOW_THREADS; - - if (ret < 0) { - pq_complete_error(self, &pgres, &error); - } - - return ret; + return 0; } diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 2066579e..48bebf4c 100644 --- a/psycopg/connection_type.c +++ b/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; - const char *c_readonly = NULL; - const char *c_deferrable = NULL; + int c_isolevel = self->isolevel; + int c_readonly = self->readonly; + 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) { return NULL; } + if (-1 == (c_autocommit = PyObject_IsTrue(autocommit))) { return NULL; } } - if (0 > conn_set_session(self, - c_isolevel, c_readonly, c_deferrable, c_autocommit)) { - return NULL; - } + self->isolevel = c_isolevel; + self->readonly = c_readonly; + 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); diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 328a2b26..8a4d78f5 100644 --- a/psycopg/pqpath.c +++ b/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; From ca59fd8b3f7f7082e72ade390914bec5d5f944bd Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 10:57:30 +0000 Subject: [PATCH 2/8] Test looking the transactions characteristics instead of the default So we test the effect, not the implementation. Tests pass on master too this way, three tests fail in this branch, related to autocommit (sort-of-obviously). --- tests/test_connection.py | 56 ++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index ea8b8f52..15d99b40 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -522,7 +522,7 @@ class IsolationLevelsTestCase(ConnectingTestCase): got_name = curs.fetchone()[0] if name is None: - curs.execute('show default_transaction_isolation;') + curs.execute('show transaction_isolation;') name = curs.fetchone()[0] self.assertEqual(name, got_name) @@ -1042,13 +1042,13 @@ class TransactionControlTests(ConnectingTestCase): cur = self.conn.cursor() self.conn.set_session( psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE) - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") self.assertEqual(cur.fetchone()[0], 'serializable') self.conn.rollback() self.conn.set_session( psycopg2.extensions.ISOLATION_LEVEL_REPEATABLE_READ) - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") if self.conn.server_version > 80000: self.assertEqual(cur.fetchone()[0], 'repeatable read') else: @@ -1057,13 +1057,13 @@ class TransactionControlTests(ConnectingTestCase): self.conn.set_session( isolation_level=psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") self.assertEqual(cur.fetchone()[0], 'read committed') self.conn.rollback() self.conn.set_session( isolation_level=psycopg2.extensions.ISOLATION_LEVEL_READ_UNCOMMITTED) - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") if self.conn.server_version > 80000: self.assertEqual(cur.fetchone()[0], 'read uncommitted') else: @@ -1073,12 +1073,12 @@ class TransactionControlTests(ConnectingTestCase): def test_set_isolation_level_str(self): cur = self.conn.cursor() self.conn.set_session("serializable") - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") self.assertEqual(cur.fetchone()[0], 'serializable') self.conn.rollback() self.conn.set_session("repeatable read") - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") if self.conn.server_version > 80000: self.assertEqual(cur.fetchone()[0], 'repeatable read') else: @@ -1086,12 +1086,12 @@ class TransactionControlTests(ConnectingTestCase): self.conn.rollback() self.conn.set_session("read committed") - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") self.assertEqual(cur.fetchone()[0], 'read committed') self.conn.rollback() self.conn.set_session("read uncommitted") - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") if self.conn.server_version > 80000: self.assertEqual(cur.fetchone()[0], 'read uncommitted') else: @@ -1106,57 +1106,57 @@ class TransactionControlTests(ConnectingTestCase): def test_set_read_only(self): cur = self.conn.cursor() self.conn.set_session(readonly=True) - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') self.conn.rollback() - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') self.conn.rollback() cur = self.conn.cursor() self.conn.set_session(readonly=None) - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') self.conn.rollback() self.conn.set_session(readonly=False) - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'off') self.conn.rollback() def test_set_default(self): cur = self.conn.cursor() - cur.execute("SHOW default_transaction_isolation;") - default_isolevel = cur.fetchone()[0] - cur.execute("SHOW default_transaction_read_only;") - default_readonly = cur.fetchone()[0] + cur.execute("SHOW transaction_isolation;") + isolevel = cur.fetchone()[0] + cur.execute("SHOW transaction_read_only;") + readonly = cur.fetchone()[0] self.conn.rollback() self.conn.set_session(isolation_level='serializable', readonly=True) self.conn.set_session(isolation_level='default', readonly='default') - cur.execute("SHOW default_transaction_isolation;") - self.assertEqual(cur.fetchone()[0], default_isolevel) - cur.execute("SHOW default_transaction_read_only;") - self.assertEqual(cur.fetchone()[0], default_readonly) + cur.execute("SHOW transaction_isolation;") + self.assertEqual(cur.fetchone()[0], isolevel) + cur.execute("SHOW transaction_read_only;") + self.assertEqual(cur.fetchone()[0], readonly) @skip_before_postgres(9, 1) def test_set_deferrable(self): cur = self.conn.cursor() self.conn.set_session(readonly=True, deferrable=True) - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') - cur.execute("SHOW default_transaction_deferrable;") + cur.execute("SHOW transaction_deferrable;") self.assertEqual(cur.fetchone()[0], 'on') self.conn.rollback() - cur.execute("SHOW default_transaction_deferrable;") + cur.execute("SHOW transaction_deferrable;") self.assertEqual(cur.fetchone()[0], 'on') self.conn.rollback() self.conn.set_session(deferrable=False) - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') - cur.execute("SHOW default_transaction_deferrable;") + cur.execute("SHOW transaction_deferrable;") self.assertEqual(cur.fetchone()[0], 'off') self.conn.rollback() @@ -1258,9 +1258,9 @@ class AutocommitTests(ConnectingTestCase): self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) self.assertEqual(self.conn.get_transaction_status(), psycopg2.extensions.TRANSACTION_STATUS_IDLE) - cur.execute("SHOW default_transaction_isolation;") + cur.execute("SHOW transaction_isolation;") self.assertEqual(cur.fetchone()[0], 'serializable') - cur.execute("SHOW default_transaction_read_only;") + cur.execute("SHOW transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'on') From 8527144173f9679b2779c71d4dbdfa0571e4bd40 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 11:20:54 +0000 Subject: [PATCH 3/8] Better separation between interface and state change code The state change function has a C callable signature. --- psycopg/connection.h | 4 +- psycopg/connection_int.c | 94 +++++----------------------------- psycopg/connection_type.c | 103 +++++++++++++++++++++++++++++++++++--- 3 files changed, 110 insertions(+), 91 deletions(-) diff --git a/psycopg/connection.h b/psycopg/connection.h index 6c39263e..00430f95 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -167,8 +167,8 @@ HIDDEN void conn_close_locked(connectionObject *self); RAISES_NEG HIDDEN int conn_commit(connectionObject *self); RAISES_NEG HIDDEN int conn_rollback(connectionObject *self); 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_set_session(connectionObject *self, int autocommit, + int isolevel, int readonly, int deferrable); 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); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index d8d693fe..da80b3aa 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -1194,9 +1194,11 @@ 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) { +static int +_adjust_isolevel(connectionObject *self, int level) { if (self->server_version < 80000) { if (level == ISOLATION_LEVEL_READ_UNCOMMITTED) { level = ISOLATION_LEVEL_READ_COMMITTED; @@ -1209,93 +1211,21 @@ static int _adjust_isolevel(connectionObject *self, int level) { } -/* parse a python object into one of the possible isolation level values */ - +/* Change the state of the session */ RAISES_NEG int -conn_parse_isolevel(connectionObject *self, PyObject *pyval) +conn_set_session(connectionObject *self, int autocommit, + int isolevel, int readonly, int deferrable) { - int rv = -1; - long level; + isolevel = _adjust_isolevel(self, isolevel); - Py_INCREF(pyval); /* for ensure_bytes */ + self->isolevel = isolevel; + self->readonly = readonly; + self->deferrable = deferrable; + self->autocommit = autocommit; - /* 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; + return 0; } -/* 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 */ diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 48bebf4c..f574b090 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -36,6 +36,9 @@ #include #include +extern HIDDEN const char *srv_isolevels[]; +extern HIDDEN const char *srv_readonly[]; +extern HIDDEN const char *srv_deferrable[]; /** DBAPI methods **/ @@ -442,6 +445,92 @@ exit: } +/* parse a python object into one of the possible isolation level values */ + +RAISES_NEG static int +_psyco_conn_parse_isolevel(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; + } + } + +exit: + Py_XDECREF(pyval); + + return rv; +} + +/* convert False/True/"default" -> 0/1/2 */ + +RAISES_NEG static int +_psyco_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; +} + /* set_session - set default transaction characteristics */ #define psyco_conn_set_session_doc \ @@ -474,13 +563,13 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) } if (Py_None != isolevel) { - if (0 > (c_isolevel = conn_parse_isolevel(self, isolevel))) { + if (0 > (c_isolevel = _psyco_conn_parse_isolevel(isolevel))) { return NULL; } } if (Py_None != readonly) { - if (0 > (c_readonly = conn_parse_onoff(readonly))) { + if (0 > (c_readonly = _psyco_conn_parse_onoff(readonly))) { return NULL; } } @@ -491,7 +580,7 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) " from PostgreSQL 9.1"); return NULL; } - if (0 > (c_deferrable = conn_parse_onoff(readonly))) { + if (0 > (c_deferrable = _psyco_conn_parse_onoff(readonly))) { return NULL; } } @@ -500,10 +589,10 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) if (-1 == (c_autocommit = PyObject_IsTrue(autocommit))) { return NULL; } } - self->isolevel = c_isolevel; - self->readonly = c_readonly; - self->deferrable = c_deferrable; - self->autocommit = c_autocommit; + if (0 > conn_set_session( + self, c_autocommit, c_isolevel, c_readonly, c_deferrable)) { + return NULL; + } Py_RETURN_NONE; } From c60682c230f8e9ef6a395be55409d549fa5d7239 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 13:24:41 +0000 Subject: [PATCH 4/8] Reuse set_session to implement autocommit, set_isolation_level --- psycopg/connection.h | 2 -- psycopg/connection_int.c | 66 ++++++++++----------------------------- psycopg/connection_type.c | 38 +++++++++++++++------- 3 files changed, 43 insertions(+), 63 deletions(-) diff --git a/psycopg/connection.h b/psycopg/connection.h index 00430f95..65efcaf0 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -166,10 +166,8 @@ 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); -HIDDEN int conn_set_autocommit(connectionObject *self, int value); RAISES_NEG HIDDEN int conn_set_session(connectionObject *self, int autocommit, int isolevel, int readonly, int deferrable); -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); RAISES_NEG HIDDEN int conn_tpc_begin(connectionObject *self, xidObject *xid); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index da80b3aa..0cb4cf38 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -1180,68 +1180,36 @@ conn_rollback(connectionObject *self) return res; } -int -conn_set_autocommit(connectionObject *self, int value) -{ - Py_BEGIN_ALLOW_THREADS; - pthread_mutex_lock(&self->lock); - - self->autocommit = value; - - pthread_mutex_unlock(&self->lock); - Py_END_ALLOW_THREADS; - - 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; -} - /* Change the state of the session */ RAISES_NEG int conn_set_session(connectionObject *self, int autocommit, int isolevel, int readonly, int deferrable) { - isolevel = _adjust_isolevel(self, isolevel); + /* Promote an isolation level to one of the levels supported by the server */ + if (self->server_version < 80000) { + if (isolevel == ISOLATION_LEVEL_READ_UNCOMMITTED) { + isolevel = ISOLATION_LEVEL_READ_COMMITTED; + } + else if (isolevel == ISOLATION_LEVEL_REPEATABLE_READ) { + isolevel = ISOLATION_LEVEL_SERIALIZABLE; + } + } + + Py_BEGIN_ALLOW_THREADS; + pthread_mutex_lock(&self->lock); self->isolevel = isolevel; self->readonly = readonly; self->deferrable = deferrable; self->autocommit = autocommit; - return 0; -} + pthread_mutex_unlock(&self->lock); + Py_END_ALLOW_THREADS; - -/* conn_switch_isolation_level - switch isolation level on the connection */ - -RAISES_NEG int -conn_switch_isolation_level(connectionObject *self, int level) -{ - if (level == 0) { - self->autocommit = 1; - } - else { - level = _adjust_isolevel(self, level); - self->isolevel = level; - self->autocommit = 0; - } - - Dprintf("conn_switch_isolation_level: switched to level %d", level); + Dprintf( + "conn_set_session: autocommit %d, isolevel %d, readonly %d, deferrable %d", + autocommit, isolevel, readonly, deferrable); return 0; } diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index f574b090..7b58e3f7 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -531,6 +531,14 @@ exit: return rv; } +#define _set_session_checks(self,what) \ +do { \ + EXC_IF_CONN_CLOSED(self); \ + EXC_IF_CONN_ASYNC(self, what); \ + EXC_IF_IN_TRANSACTION(self, what); \ + EXC_IF_TPC_PREPARED(self, what); \ +} while(0) + /* set_session - set default transaction characteristics */ #define psyco_conn_set_session_doc \ @@ -553,9 +561,7 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) static char *kwlist[] = {"isolation_level", "readonly", "deferrable", "autocommit", NULL}; - EXC_IF_CONN_CLOSED(self); - EXC_IF_CONN_ASYNC(self, set_session); - EXC_IF_IN_TRANSACTION(self, set_session); + _set_session_checks(self, set_session); if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOOO", kwlist, &isolevel, &readonly, &deferrable, &autocommit)) { @@ -615,9 +621,7 @@ _psyco_conn_autocommit_set_checks(connectionObject *self) { /* wrapper to use the EXC_IF macros. * return NULL in case of error, else whatever */ - EXC_IF_CONN_CLOSED(self); - EXC_IF_CONN_ASYNC(self, autocommit); - EXC_IF_IN_TRANSACTION(self, autocommit); + _set_session_checks(self, autocommit); return Py_None; /* borrowed */ } @@ -628,7 +632,10 @@ psyco_conn_autocommit_set(connectionObject *self, PyObject *pyvalue) if (!_psyco_conn_autocommit_set_checks(self)) { return -1; } if (-1 == (value = PyObject_IsTrue(pyvalue))) { return -1; } - if (0 != conn_set_autocommit(self, value)) { return -1; } + if (0 > conn_set_session(self, value, + self->isolevel, self->readonly, self->deferrable)) { + return -1; + } return 0; } @@ -660,9 +667,7 @@ psyco_conn_set_isolation_level(connectionObject *self, PyObject *args) { int level = 1; - EXC_IF_CONN_CLOSED(self); - EXC_IF_CONN_ASYNC(self, set_isolation_level); - EXC_IF_TPC_PREPARED(self, set_isolation_level); + _set_session_checks(self, set_isolation_level); if (!PyArg_ParseTuple(args, "i", &level)) return NULL; @@ -672,8 +677,17 @@ psyco_conn_set_isolation_level(connectionObject *self, PyObject *args) return NULL; } - if (conn_switch_isolation_level(self, level) < 0) { - return NULL; + if (level == 0) { + if (0 > conn_set_session(self, 1, + ISOLATION_LEVEL_DEFAULT, self->readonly, self->deferrable)) { + return NULL; + } + } + else { + if (0 > conn_set_session(self, 0, + level, self->readonly, self->deferrable)) { + return NULL; + } } Py_RETURN_NONE; From 665e9dc665cf72086c00871756c48e8d38bade2f Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 14:43:54 +0000 Subject: [PATCH 5/8] Exposing ISOLATION_LEVEL_DEFAULT to Python This is now the state that is returned to Python if nothing has been explicitly set. --- doc/src/extensions.rst | 9 +++++++++ lib/extensions.py | 1 + tests/test_connection.py | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index 8d70ba38..2475d7b6 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -651,6 +651,15 @@ set to one of the following constants: .. __: http://www.postgresql.org/docs/current/static/transaction-iso.html#XACT-SERIALIZABLE +.. data:: ISOLATION_LEVEL_DEFAULT + + Whatever is defined by the server, either via server configuration or by + statements executed within the session outside Pyscopg control; Psycopg + will not force an isolation level of its own. If you want to know what the + value is you can use a query such as :sql:`show transaction_isolation` or + :sql:`show default_transaction_isolation`. + + .. versionadded:: 2.7 .. index:: diff --git a/lib/extensions.py b/lib/extensions.py index b123e881..f4dc706f 100644 --- a/lib/extensions.py +++ b/lib/extensions.py @@ -72,6 +72,7 @@ ISOLATION_LEVEL_READ_UNCOMMITTED = 4 ISOLATION_LEVEL_READ_COMMITTED = 1 ISOLATION_LEVEL_REPEATABLE_READ = 2 ISOLATION_LEVEL_SERIALIZABLE = 3 +ISOLATION_LEVEL_DEFAULT = 5 """psycopg connection status values.""" diff --git a/tests/test_connection.py b/tests/test_connection.py index 15d99b40..5b304ee4 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -488,7 +488,7 @@ class IsolationLevelsTestCase(ConnectingTestCase): conn = self.connect() self.assertEqual( conn.isolation_level, - psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) + psycopg2.extensions.ISOLATION_LEVEL_DEFAULT) def test_encoding(self): conn = self.connect() From 9054eeccc0460908865e5462973fd82c148a9315 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 15:19:41 +0000 Subject: [PATCH 6/8] Set default_transaction_* GUC if session state is changed in autocomit --- psycopg/connection_int.c | 75 +++++++++++++++++++++++++++++++++++++-- psycopg/connection_type.c | 2 +- psycopg/pqpath.c | 6 ++-- tests/test_connection.py | 49 ++++++++++++------------- 4 files changed, 102 insertions(+), 30 deletions(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 0cb4cf38..e6906eb5 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -41,7 +41,7 @@ const char *srv_isolevels[] = { "REPEATABLE READ", "SERIALIZABLE", "READ UNCOMMITTED", - "" /* default */ + "default" /* only to set GUC, not for BEGIN */ }; /* Read only false, true */ @@ -58,6 +58,15 @@ const char *srv_deferrable[] = { "" /* default */ }; +/* On/Off/Default GUC states + */ +const char *srv_state_guc[] = { + "off", + "on", + "default" +}; + + /* Return a new "string" from a char* from the database. * * On Py2 just get a string, on Py3 decode it in the connection codec. @@ -1186,6 +1195,10 @@ RAISES_NEG int conn_set_session(connectionObject *self, int autocommit, int isolevel, int readonly, int deferrable) { + int rv = -1; + PGresult *pgres = NULL; + char *error = NULL; + /* Promote an isolation level to one of the levels supported by the server */ if (self->server_version < 80000) { if (isolevel == ISOLATION_LEVEL_READ_UNCOMMITTED) { @@ -1199,19 +1212,75 @@ conn_set_session(connectionObject *self, int autocommit, Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&self->lock); + if (autocommit) { + /* we are in autocommit state, so no BEGIN will be issued: + * configure the session with the characteristics requested */ + if (isolevel != self->isolevel) { + if (0 > pq_set_guc_locked(self, + "default_transaction_isolation", srv_isolevels[isolevel], + &pgres, &error, &_save)) { + goto endlock; + } + } + if (readonly != self->readonly) { + if (0 > pq_set_guc_locked(self, + "default_transaction_read_only", srv_state_guc[readonly], + &pgres, &error, &_save)) { + goto endlock; + } + } + if (deferrable != self->deferrable && self->server_version >= 90100) { + if (0 > pq_set_guc_locked(self, + "default_transaction_deferrable", srv_state_guc[deferrable], + &pgres, &error, &_save)) { + goto endlock; + } + } + } + else if (self->autocommit) { + /* we are moving from autocommit to not autocommit, so revert the + * characteristics to defaults to let BEGIN do its work */ + if (0 > pq_set_guc_locked(self, + "default_transaction_isolation", "default", + &pgres, &error, &_save)) { + goto endlock; + } + if (0 > pq_set_guc_locked(self, + "default_transaction_read_only", "default", + &pgres, &error, &_save)) { + goto endlock; + } + if (self->server_version >= 90100) { + if (0 > pq_set_guc_locked(self, + "default_transaction_deferrable", "default", + &pgres, &error, &_save)) { + goto endlock; + } + } + } + + self->autocommit = autocommit; self->isolevel = isolevel; self->readonly = readonly; self->deferrable = deferrable; - self->autocommit = autocommit; + rv = 0; +endlock: pthread_mutex_unlock(&self->lock); Py_END_ALLOW_THREADS; + if (rv < 0) { + pq_complete_error(self, &pgres, &error); + goto exit; + } + Dprintf( "conn_set_session: autocommit %d, isolevel %d, readonly %d, deferrable %d", autocommit, isolevel, readonly, deferrable); - return 0; + +exit: + return rv; } diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 7b58e3f7..7647056b 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -671,7 +671,7 @@ psyco_conn_set_isolation_level(connectionObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "i", &level)) return NULL; - if (level < 0 || level > 4) { + if (level < 0 || level > 5) { PyErr_SetString(PyExc_ValueError, "isolation level must be between 0 and 4"); return NULL; diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 8a4d78f5..c686402d 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -495,8 +495,10 @@ pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error, 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], + (conn->isolevel >= 1 && conn->isolevel <= 4) + ? " ISOLATION LEVEL " : "", + (conn->isolevel >= 1 && conn->isolevel <= 4) + ? srv_isolevels[conn->isolevel] : "", srv_readonly[conn->readonly], srv_deferrable[conn->deferrable]); diff --git a/tests/test_connection.py b/tests/test_connection.py index 5b304ee4..314aa075 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -529,7 +529,26 @@ class IsolationLevelsTestCase(ConnectingTestCase): conn.commit() self.assertRaises(ValueError, conn.set_isolation_level, -1) - self.assertRaises(ValueError, conn.set_isolation_level, 5) + self.assertRaises(ValueError, conn.set_isolation_level, 6) + + def test_set_isolation_level_default(self): + conn = self.connect() + curs = conn.cursor() + + conn.autocommit = True + curs.execute("set default_transaction_isolation to 'read committed'") + + conn.autocommit = False + conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE) + self.assertEqual(conn.isolation_level, + psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE) + curs.execute("show transaction_isolation") + self.assertEqual(curs.fetchone()[0], "serializable") + + conn.rollback() + conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_DEFAULT) + curs.execute("show transaction_isolation") + self.assertEqual(curs.fetchone()[0], "read committed") def test_set_isolation_level_abort(self): conn = self.connect() @@ -541,32 +560,14 @@ class IsolationLevelsTestCase(ConnectingTestCase): self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_INTRANS, conn.get_transaction_status()) - conn.set_isolation_level( + # changed in psycopg 2.7 + self.assertRaises(psycopg2.ProgrammingError, + conn.set_isolation_level, psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE) - self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_IDLE, - conn.get_transaction_status()) - cur.execute("select count(*) from isolevel;") - self.assertEqual(0, cur.fetchone()[0]) - - cur.execute("insert into isolevel values (10);") self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_INTRANS, conn.get_transaction_status()) - conn.set_isolation_level( - psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) - self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_IDLE, - conn.get_transaction_status()) - cur.execute("select count(*) from isolevel;") - self.assertEqual(0, cur.fetchone()[0]) - - cur.execute("insert into isolevel values (10);") - self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_IDLE, - conn.get_transaction_status()) - conn.set_isolation_level( - psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) - self.assertEqual(psycopg2.extensions.TRANSACTION_STATUS_IDLE, - conn.get_transaction_status()) - cur.execute("select count(*) from isolevel;") - self.assertEqual(1, cur.fetchone()[0]) + self.assertEqual(conn.isolation_level, + psycopg2.extensions.ISOLATION_LEVEL_DEFAULT) def test_isolation_level_autocommit(self): cnn1 = self.connect() From c54a614c6e479e64d1819e2122a5b8972adc7a40 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 Feb 2017 15:55:59 +0000 Subject: [PATCH 7/8] Added documentation about the changes in transaction control --- NEWS | 9 ++++++ doc/src/connection.rst | 64 ++++++++++++++++++++++++++++++++++++------ doc/src/extensions.rst | 24 ++++++++-------- doc/src/usage.rst | 3 +- 4 files changed, 79 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 499656d2..201e5e5a 100644 --- a/NEWS +++ b/NEWS @@ -33,6 +33,10 @@ New features: (:ticket:`#491`). - Added ``async_`` as an alias for ``async`` to support Python 3.7 where ``async`` will become a keyword (:ticket:`#495`). +- Unless in autocommit, do not use :sql:`default_transaction_*` settings to + control the session characteristics as it may create problems with external + connection pools such as pgbouncer; use :sql:`BEGIN` options instead + (:ticket:`#503`). Bug fixes: @@ -44,6 +48,11 @@ Other changes: - Dropped support for Python 2.5. - Dropped support for client library older than PostgreSQL 9.1 (but older server versions are still supported). +- `~connection.isolation_level` doesn't read from the database but will return + `~psycopg2.extensions.ISOLATION_LEVEL_DEFAULT` if no value was set on the + connection. +- `~connection.set_isolation_level()` will throw an exception if executed + inside a transaction; previously it would have silently rolled it back. What's new in psycopg 2.6.3 diff --git a/doc/src/connection.rst b/doc/src/connection.rst index 0bc584ca..2adad597 100644 --- a/doc/src/connection.rst +++ b/doc/src/connection.rst @@ -400,6 +400,32 @@ The ``connection`` class .. versionadded:: 2.4.2 + .. versionchanged:: 2.7 + Before this version, the function would have set + :sql:`default_transaction_*` attribute in the current session; + this implementation has the problem of not playing well with + external connection pooling working at transaction level and not + resetting the state of the session: changing the default + transaction would pollute the connections in the pool and create + problems to other applications using the same pool. + + Starting from 2.7, if the connection is not autocommit, the + transaction characteristics are issued together with :sql:`BEGIN` + and will leave the :sql:`default_transaction_*` settings untouched. + For example:: + + conn.set_session(readonly=True) + + will not change :sql:`default_transaction_read_only`, but + following transaction will start with a :sql:`BEGIN READ ONLY`. + Conversely, using:: + + conn.set_session(readonly=True, autocommit=True) + + will set :sql:`default_transaction_read_only` to :sql:`on` and + rely on the server to apply the read only state to whatever + transaction, implicit or explicit, is executed in the connection. + .. attribute:: autocommit @@ -428,32 +454,54 @@ The ``connection`` class .. versionadded:: 2.4.2 - .. attribute:: isolation_level .. method:: set_isolation_level(level) .. note:: - From version 2.4.2, `set_session()` and `autocommit`, offer + From version 2.4.2, `set_session()` and `autocommit` offer finer control on the transaction characteristics. - Read or set the `transaction isolation level`_ for the current session. + Set the `transaction isolation level`_ for the current session. The level defines the different phenomena that can happen in the database between concurrent transactions. - The value set or read is an integer: symbolic constants are defined in + The value set is an integer: symbolic constants are defined in the module `psycopg2.extensions`: see :ref:`isolation-level-constants` for the available values. - The default level is :sql:`READ COMMITTED`: at this level a - transaction is automatically started the first time a database command - is executed. If you want an *autocommit* mode, switch to - `~psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT` before + The default level is `~psycopg2.extensions.ISOLATION_LEVEL_DEFAULT`: + at this level a transaction is automatically started the first time a + database command is executed. If you want an *autocommit* mode, + switch to `~psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT` before executing any command:: >>> conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) See also :ref:`transactions-control`. + .. versionchanged:: 2.7 + + the function must be called outside a transaction; previously it + would have executed an implicit :sql:`ROLLBACK`; it will now raise + an exception. + + + .. attribute:: isolation_level + + Read the `transaction isolation level`_ for the current session. The + value is one of the :ref:`isolation-level-constants` defined in the + `psycopg2.extensions` module. + + .. versionchanged:: 2.7 + + the default value for `!isolation_level` is + `~psycopg2.extensions.ISOLATION_LEVEL_DEFAULT`; previously the + property would have queried the server and returned the real value + applied. To know this value you can run a query such as :sql:`show + transaction_isolation`. Usually the default value is `READ + COMMITTED`, but this may be changed in the server configuration. + + .. index:: pair: Client; Encoding diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index 2475d7b6..ae40b720 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -567,15 +567,16 @@ Isolation level constants ------------------------- Psycopg2 `connection` objects hold informations about the PostgreSQL -`transaction isolation level`_. The current transaction level can be read -from the `~connection.isolation_level` attribute. The default isolation -level is :sql:`READ COMMITTED`. A different isolation level con be set -through the `~connection.set_isolation_level()` method. The level can be -set to one of the following constants: +`transaction isolation level`_. By default Psycopg doesn't change the default +configuration of the server (`ISOLATION_LEVEL_DEFAULT`); the default for +PostgreSQL servers is typically :sql:`READ COMMITTED`, but this may be changed +in the server configuration files. A different isolation level can be set +through the `~connection.set_isolation_level()` or `~connection.set_session()` +methods. The level can be set to one of the following constants: .. data:: ISOLATION_LEVEL_AUTOCOMMIT - No transaction is started when command are issued and no + No transaction is started when commands are executed and no `~connection.commit()` or `~connection.rollback()` is required. Some PostgreSQL command such as :sql:`CREATE DATABASE` or :sql:`VACUUM` can't run into a transaction: to run such command use:: @@ -653,11 +654,12 @@ set to one of the following constants: .. data:: ISOLATION_LEVEL_DEFAULT - Whatever is defined by the server, either via server configuration or by - statements executed within the session outside Pyscopg control; Psycopg - will not force an isolation level of its own. If you want to know what the - value is you can use a query such as :sql:`show transaction_isolation` or - :sql:`show default_transaction_isolation`. + A new transaction is started at the first `~cursor.execute()` command, but + the isolation level is not explicitly selected by Psycopg: the server will + use whatever level is defined in its configuration or by statements + executed within the session outside Pyscopg control. If you want to know + what the value is you can use a query such as :sql:`show + transaction_isolation`. .. versionadded:: 2.7 diff --git a/doc/src/usage.rst b/doc/src/usage.rst index 1366485a..6cb038b6 100644 --- a/doc/src/usage.rst +++ b/doc/src/usage.rst @@ -676,8 +676,7 @@ commands executed will be immediately committed and no rollback is possible. A few commands (e.g. :sql:`CREATE DATABASE`, :sql:`VACUUM`...) require to be run outside any transaction: in order to be able to run these commands from Psycopg, the connection must be in autocommit mode: you can use the -`~connection.autocommit` property (`~connection.set_isolation_level()` in -older versions). +`~connection.autocommit` property. .. warning:: From 7485fabe4fb460fdab33296cc86fc33e17aaef47 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 7 Feb 2017 00:40:08 +0000 Subject: [PATCH 8/8] Fixed BEGIN; SET TRANSACTION with PG 7.4 --- psycopg/pqpath.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index c686402d..50bd5201 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -493,14 +493,22 @@ pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error, return 0; } - snprintf(buf, bufsize, "BEGIN%s%s%s%s%s", - conn->server_version < 80000 ? ";SET TRANSACTION" : "", - (conn->isolevel >= 1 && conn->isolevel <= 4) - ? " ISOLATION LEVEL " : "", - (conn->isolevel >= 1 && conn->isolevel <= 4) - ? srv_isolevels[conn->isolevel] : "", - srv_readonly[conn->readonly], - srv_deferrable[conn->deferrable]); + if (conn->isolevel == ISOLATION_LEVEL_DEFAULT + && conn->readonly == STATE_DEFAULT + && conn->deferrable == STATE_DEFAULT) { + strcpy(buf, "BEGIN"); + } + else { + snprintf(buf, bufsize, + conn->server_version >= 80000 ? + "BEGIN%s%s%s%s" : "BEGIN;SET TRANSACTION%s%s%s%s", + (conn->isolevel >= 1 && conn->isolevel <= 4) + ? " ISOLATION LEVEL " : "", + (conn->isolevel >= 1 && conn->isolevel <= 4) + ? 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)