From 18d67a73d56573480e537c518f077951adaffc03 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 21 Aug 2014 05:01:34 +0100 Subject: [PATCH 1/5] Added test to verify withhold transaction behaviour A withhold cursor can read its data when the transaction is closed, so it shouldn't start a new one upon movement/close. --- tests/test_cursor.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 8470adec..2f92b6e4 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -176,10 +176,7 @@ class CursorTests(ConnectingTestCase): curs.execute("select data from invname order by data") self.assertEqual(curs.fetchall(), [(10,), (20,), (30,)]) - def test_withhold(self): - self.assertRaises(psycopg2.ProgrammingError, self.conn.cursor, - withhold=True) - + def _create_withhold_table(self): curs = self.conn.cursor() try: curs.execute("drop table withhold") @@ -190,6 +187,11 @@ class CursorTests(ConnectingTestCase): curs.execute("insert into withhold values (%s)", (i,)) curs.close() + def test_withhold(self): + self.assertRaises(psycopg2.ProgrammingError, self.conn.cursor, + withhold=True) + + self._create_withhold_table() curs = self.conn.cursor("W") self.assertEqual(curs.withhold, False); curs.withhold = True @@ -209,6 +211,30 @@ class CursorTests(ConnectingTestCase): curs.execute("drop table withhold") self.conn.commit() + def test_withhold_no_begin(self): + self._create_withhold_table() + curs = self.conn.cursor("w", withhold=True) + curs.execute("select data from withhold order by data") + self.assertEqual(curs.fetchone(), (10,)) + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_BEGIN) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_INTRANS) + + self.conn.commit() + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + + self.assertEqual(curs.fetchone(), (20,)) + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + + curs.close() + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + def test_scrollable(self): self.assertRaises(psycopg2.ProgrammingError, self.conn.cursor, scrollable=True) From d131b2b22bec228afd6c15ed679f64965b1fa96b Mon Sep 17 00:00:00 2001 From: Alexey Borzenkov Date: Tue, 9 Jul 2013 23:36:04 +0400 Subject: [PATCH 2/5] No implicit transaction on named cursor close Also, don't start an implicit transaction when fetching with named with hold cursor, since it already returns results from a previously committed transaction. --- psycopg/cursor_type.c | 20 ++++++++++---------- psycopg/pqpath.c | 4 ++-- psycopg/pqpath.h | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index ddbed298..8df1df2a 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -63,7 +63,7 @@ psyco_curs_close(cursorObject *self) EXC_IF_NO_MARK(self); PyOS_snprintf(buffer, 127, "CLOSE \"%s\"", self->name); - if (pq_execute(self, buffer, 0, 0) == -1) return NULL; + if (pq_execute(self, buffer, 0, 0, 1) == -1) return NULL; } self->closed = 1; @@ -444,7 +444,7 @@ _psyco_curs_execute(cursorObject *self, /* At this point, the SQL statement must be str, not unicode */ - tmp = pq_execute(self, Bytes_AS_STRING(self->query), async, no_result); + tmp = pq_execute(self, Bytes_AS_STRING(self->query), async, no_result, 0); Dprintf("psyco_curs_execute: res = %d, pgres = %p", tmp, self->pgres); if (tmp < 0) { goto exit; } @@ -766,7 +766,7 @@ psyco_curs_fetchone(cursorObject *self) EXC_IF_ASYNC_IN_PROGRESS(self, fetchone); EXC_IF_TPC_PREPARED(self->conn, fetchone); PyOS_snprintf(buffer, 127, "FETCH FORWARD 1 FROM \"%s\"", self->name); - if (pq_execute(self, buffer, 0, 0) == -1) return NULL; + if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -816,7 +816,7 @@ psyco_curs_next_named(cursorObject *self) PyOS_snprintf(buffer, 127, "FETCH FORWARD %ld FROM \"%s\"", self->itersize, self->name); - if (pq_execute(self, buffer, 0, 0) == -1) return NULL; + if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -885,7 +885,7 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) EXC_IF_TPC_PREPARED(self->conn, fetchone); PyOS_snprintf(buffer, 127, "FETCH FORWARD %d FROM \"%s\"", (int)size, self->name); - if (pq_execute(self, buffer, 0, 0) == -1) { goto exit; } + if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) { goto exit; } if (_psyco_curs_prefetch(self) < 0) { goto exit; } } @@ -960,7 +960,7 @@ psyco_curs_fetchall(cursorObject *self) EXC_IF_ASYNC_IN_PROGRESS(self, fetchall); EXC_IF_TPC_PREPARED(self->conn, fetchall); PyOS_snprintf(buffer, 127, "FETCH FORWARD ALL FROM \"%s\"", self->name); - if (pq_execute(self, buffer, 0, 0) == -1) { goto exit; } + if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) { goto exit; } if (_psyco_curs_prefetch(self) < 0) { goto exit; } } @@ -1178,7 +1178,7 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) else { PyOS_snprintf(buffer, 127, "MOVE %d FROM \"%s\"", value, self->name); } - if (pq_execute(self, buffer, 0, 0) == -1) return NULL; + if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -1391,7 +1391,7 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(file); self->copyfile = file; - if (pq_execute(self, query, 0, 0) >= 0) { + if (pq_execute(self, query, 0, 0, 0) >= 0) { res = Py_None; Py_INCREF(Py_None); } @@ -1485,7 +1485,7 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(file); self->copyfile = file; - if (pq_execute(self, query, 0, 0) >= 0) { + if (pq_execute(self, query, 0, 0, 0) >= 0) { res = Py_None; Py_INCREF(Py_None); } @@ -1559,7 +1559,7 @@ psyco_curs_copy_expert(cursorObject *self, PyObject *args, PyObject *kwargs) self->copyfile = file; /* At this point, the SQL statement must be str, not unicode */ - if (pq_execute(self, Bytes_AS_STRING(sql), 0, 0) >= 0) { + if (pq_execute(self, Bytes_AS_STRING(sql), 0, 0, 0) >= 0) { res = Py_None; Py_INCREF(res); } diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 42460f5b..3512f639 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -893,7 +893,7 @@ pq_flush(connectionObject *conn) */ RAISES_NEG int -pq_execute(cursorObject *curs, const char *query, int async, int no_result) +pq_execute(cursorObject *curs, const char *query, int async, int no_result, int no_begin) { PGresult *pgres = NULL; char *error = NULL; @@ -916,7 +916,7 @@ pq_execute(cursorObject *curs, const char *query, int async, int no_result) Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&(curs->conn->lock)); - if (pq_begin_locked(curs->conn, &pgres, &error, &_save) < 0) { + if (!no_begin && pq_begin_locked(curs->conn, &pgres, &error, &_save) < 0) { pthread_mutex_unlock(&(curs->conn->lock)); Py_BLOCK_THREADS; pq_complete_error(curs->conn, &pgres, &error); diff --git a/psycopg/pqpath.h b/psycopg/pqpath.h index 40beea19..bd3293f8 100644 --- a/psycopg/pqpath.h +++ b/psycopg/pqpath.h @@ -36,7 +36,7 @@ HIDDEN PGresult *pq_get_last_result(connectionObject *conn); RAISES_NEG HIDDEN int pq_fetch(cursorObject *curs, int no_result); RAISES_NEG HIDDEN int pq_execute(cursorObject *curs, const char *query, - int async, int no_result); + int async, int no_result, int no_begin); HIDDEN int pq_send_query(connectionObject *conn, const char *query); HIDDEN int pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); From 75a0b2ffe282d3ac83e9a6c00b03f4c09f98b7fc Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 21 Aug 2014 05:14:20 +0100 Subject: [PATCH 3/5] Added test to verify withhold cursors work in autocommit --- tests/test_cursor.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 2f92b6e4..07956554 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -235,6 +235,28 @@ class CursorTests(ConnectingTestCase): self.assertEqual(self.conn.get_transaction_status(), psycopg2.extensions.TRANSACTION_STATUS_IDLE) + def test_withhold_autocommit(self): + self._create_withhold_table() + self.conn.commit() + self.conn.autocommit = True + curs = self.conn.cursor("w", withhold=True) + curs.execute("select data from withhold order by data") + + self.assertEqual(curs.fetchone(), (10,)) + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + + self.conn.commit() + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + + curs.close() + self.assertEqual(self.conn.status, psycopg2.extensions.STATUS_READY) + self.assertEqual(self.conn.get_transaction_status(), + psycopg2.extensions.TRANSACTION_STATUS_IDLE) + def test_scrollable(self): self.assertRaises(psycopg2.ProgrammingError, self.conn.cursor, scrollable=True) From e08be44db70efda06bb8397a911908cad9e9f12d Mon Sep 17 00:00:00 2001 From: Alexey Borzenkov Date: Tue, 9 Jul 2013 23:29:58 +0400 Subject: [PATCH 4/5] Allow using named with hold cursors in autocommit --- psycopg/cursor_type.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 8df1df2a..5650e437 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -478,7 +478,7 @@ psyco_curs_execute(cursorObject *self, PyObject *args, PyObject *kwargs) "can't call .execute() on named cursors more than once"); return NULL; } - if (self->conn->autocommit) { + if (self->conn->autocommit && !self->withhold) { psyco_set_error(ProgrammingError, self, "can't use a named cursor outside of transactions"); return NULL; From c114aff4356be1b9b3fa1ed2c3e123926cffe6db Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 21 Aug 2014 05:27:57 +0100 Subject: [PATCH 5/5] Document WITH HOLD corrections. --- NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS b/NEWS index e471e528..2f71a374 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,9 @@ What's new in psycopg 2.5.4 proper methods (:ticket:`#219`). - Force conversion of pool arguments to integer to avoid potentially unbounded pools (:ticket:`#220`). +- Cursors :sql:`WITH HOLD` don't begin a new transaction upon move/fetch/close + (:ticket:`#228`). +- Cursors :sql:`WITH HOLD` can be used in autocommit (:ticket:`#229`). - Don't ignore silently the `cursor.callproc` argument without a length. - Added a few errors missing from `~psycopg2.errorcodes`, defined by PostgreSQL but not documented.