From 8aaf8f190243a75db3e9239cde0efd2736543184 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 11 Jan 2018 01:59:49 +0000 Subject: [PATCH] Fixed idempotence check changing connection characteristics --- NEWS | 2 ++ psycopg/connection_int.c | 31 ++++++++++++++++++++++--------- psycopg/connection_type.c | 27 ++++++++++++++------------- tests/test_connection.py | 10 ++++++++++ 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/NEWS b/NEWS index adb5d3aa..0fb41199 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ What's new in psycopg 2.7.4 - Fixed Solaris 10 support (:ticket:`#532`). - `cursor.mogrify()` can be called on closed cursors (:ticket:`#579`). +- Fixed setting session characteristics in corner cases on autocommit + connections (:ticket:`#580`). - Fixed `~psycopg2.extras.MinTimeLoggingCursor` on Python 3 (:ticket:`#609`). - Fixed parsing of array of points as floats (:ticket:`#613`). - Fixed `~psycopg2.__libpq_version__` building with libpq >= 10.1 diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index e8081b9e..3ea5ca32 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -67,6 +67,9 @@ const char *srv_state_guc[] = { }; +const int SRV_STATE_UNCHANGED = -1; + + /* Return a new "string" from a char* from the database. * * On Py2 just get a string, on Py3 decode it in the connection codec. @@ -1188,6 +1191,8 @@ conn_set_session(connectionObject *self, int autocommit, int rv = -1; PGresult *pgres = NULL; char *error = NULL; + int want_autocommit = autocommit == SRV_STATE_UNCHANGED ? + self->autocommit : autocommit; if (deferrable != self->deferrable && self->server_version < 90100) { PyErr_SetString(ProgrammingError, @@ -1209,24 +1214,24 @@ 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: + if (want_autocommit) { + /* we are or are going in autocommit state, so no BEGIN will be issued: * configure the session with the characteristics requested */ - if (isolevel != self->isolevel) { + if (isolevel != SRV_STATE_UNCHANGED) { if (0 > pq_set_guc_locked(self, "default_transaction_isolation", srv_isolevels[isolevel], &pgres, &error, &_save)) { goto endlock; } } - if (readonly != self->readonly) { + if (readonly != SRV_STATE_UNCHANGED) { if (0 > pq_set_guc_locked(self, "default_transaction_read_only", srv_state_guc[readonly], &pgres, &error, &_save)) { goto endlock; } } - if (deferrable != self->deferrable) { + if (deferrable != SRV_STATE_UNCHANGED) { if (0 > pq_set_guc_locked(self, "default_transaction_deferrable", srv_state_guc[deferrable], &pgres, &error, &_save)) { @@ -1260,10 +1265,18 @@ conn_set_session(connectionObject *self, int autocommit, } } - self->autocommit = autocommit; - self->isolevel = isolevel; - self->readonly = readonly; - self->deferrable = deferrable; + if (autocommit != SRV_STATE_UNCHANGED) { + self->autocommit = autocommit; + } + if (isolevel != SRV_STATE_UNCHANGED) { + self->isolevel = isolevel; + } + if (readonly != SRV_STATE_UNCHANGED) { + self->readonly = readonly; + } + if (deferrable != SRV_STATE_UNCHANGED) { + self->deferrable = deferrable; + } rv = 0; endlock: diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 8c5085b5..6a66d48d 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -39,6 +39,7 @@ extern HIDDEN const char *srv_isolevels[]; extern HIDDEN const char *srv_readonly[]; extern HIDDEN const char *srv_deferrable[]; +extern HIDDEN const int SRV_STATE_UNCHANGED; /** DBAPI methods **/ @@ -561,10 +562,10 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) PyObject *deferrable = Py_None; PyObject *autocommit = Py_None; - int c_isolevel = self->isolevel; - int c_readonly = self->readonly; - int c_deferrable = self->deferrable; - int c_autocommit = self->autocommit; + int c_isolevel = SRV_STATE_UNCHANGED; + int c_readonly = SRV_STATE_UNCHANGED; + int c_deferrable = SRV_STATE_UNCHANGED; + int c_autocommit = SRV_STATE_UNCHANGED; static char *kwlist[] = {"isolation_level", "readonly", "deferrable", "autocommit", NULL}; @@ -637,7 +638,7 @@ psyco_conn_autocommit_set(connectionObject *self, PyObject *pyvalue) if (!_psyco_set_session_check_setter_wrapper(self)) { return -1; } if (-1 == (value = PyObject_IsTrue(pyvalue))) { return -1; } if (0 > conn_set_session(self, value, - self->isolevel, self->readonly, self->deferrable)) { + SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED)) { return -1; } @@ -668,8 +669,8 @@ psyco_conn_isolation_level_set(connectionObject *self, PyObject *pyvalue) if (!_psyco_set_session_check_setter_wrapper(self)) { return -1; } if (0 > (value = _psyco_conn_parse_isolevel(pyvalue))) { return -1; } - if (0 > conn_set_session(self, self->autocommit, - value, self->readonly, self->deferrable)) { + if (0 > conn_set_session(self, SRV_STATE_UNCHANGED, + value, SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED)) { return -1; } @@ -715,13 +716,13 @@ psyco_conn_set_isolation_level(connectionObject *self, PyObject *args) if (level == 0) { if (0 > conn_set_session(self, 1, - self->isolevel, self->readonly, self->deferrable)) { + SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED)) { return NULL; } } else { if (0 > conn_set_session(self, 0, - level, self->readonly, self->deferrable)) { + level, SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED)) { return NULL; } } @@ -767,8 +768,8 @@ psyco_conn_readonly_set(connectionObject *self, PyObject *pyvalue) if (!_psyco_set_session_check_setter_wrapper(self)) { return -1; } if (0 > (value = _psyco_conn_parse_onoff(pyvalue))) { return -1; } - if (0 > conn_set_session(self, self->autocommit, - self->isolevel, value, self->deferrable)) { + if (0 > conn_set_session(self, SRV_STATE_UNCHANGED, + SRV_STATE_UNCHANGED, value, SRV_STATE_UNCHANGED)) { return -1; } @@ -813,8 +814,8 @@ psyco_conn_deferrable_set(connectionObject *self, PyObject *pyvalue) if (!_psyco_set_session_check_setter_wrapper(self)) { return -1; } if (0 > (value = _psyco_conn_parse_onoff(pyvalue))) { return -1; } - if (0 > conn_set_session(self, self->autocommit, - self->isolevel, self->readonly, value)) { + if (0 > conn_set_session(self, SRV_STATE_UNCHANGED, + SRV_STATE_UNCHANGED, SRV_STATE_UNCHANGED, value)) { return -1; } diff --git a/tests/test_connection.py b/tests/test_connection.py index eff10650..21aec41e 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1382,6 +1382,16 @@ class TransactionControlTests(ConnectingTestCase): cur.execute("SHOW default_transaction_read_only;") self.assertEqual(cur.fetchone()[0], 'off') + def test_idempotence_check(self): + self.conn.autocommit = False + self.conn.readonly = True + self.conn.autocommit = True + self.conn.readonly = True + + cur = self.conn.cursor() + cur.execute("SHOW transaction_read_only") + self.assertEqual(cur.fetchone()[0], 'on') + class AutocommitTests(ConnectingTestCase): def test_closed(self):