From 6d8f4f9f0d357beceb4a112e0ba9e3c1f314b8bb Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 21 Jul 2018 18:32:02 +0100 Subject: Close named cursor if exist, even if we didn't run execute Close #746 --- NEWS | 7 +++++++ psycopg/cursor_type.c | 47 ++++++++++++++++++++++++++++++++++++++--------- tests/test_cursor.py | 13 +++++++++++++ 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 54237b8..e0c4da5 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,13 @@ Other changes: install``. +What's new in psycopg 2.7.6 +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- Close named cursors if exist, even if `~cursor.execute()` wasn't called + (:ticket:`#746`). + + What's new in psycopg 2.7.5 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index d73bc3a..14647a4 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -49,21 +49,22 @@ static PyObject * psyco_curs_close(cursorObject *self) { + PyObject *rv = NULL; + char *lname = NULL; + EXC_IF_ASYNC_IN_PROGRESS(self, close); if (self->closed) { + rv = Py_None; + Py_INCREF(rv); goto exit; } if (self->qname != NULL) { - char buffer[128]; + const int bufsize = 255; + char buffer[bufsize + 1]; PGTransactionStatusType status; - if (!self->query) { - Dprintf("skipping named cursor close because unused"); - goto close; - } - if (self->conn) { status = PQtransactionStatus(self->conn->pgconn); } @@ -77,17 +78,45 @@ psyco_curs_close(cursorObject *self) goto close; } + /* We should close a server-side cursor only if exists, or we get an + * error (#716). If we execute()d the cursor should exist alright, but + * if we didn't there is still the expectation that the cursor is + * closed (#746). + * + * So if we didn't execute() check for the cursor existence before + * closing it (the view exists since PG 8.2 according to docs). + */ + if (!self->query && self->conn->server_version >= 80200) { + if (!(lname = psycopg_escape_string( + self->conn, self->name, -1, NULL, NULL))) { + goto exit; + } + PyOS_snprintf(buffer, bufsize, + "SELECT 1 FROM pg_catalog.pg_cursors where name = %s", + lname); + if (pq_execute(self, buffer, 0, 0, 1) == -1) { goto exit; } + + if (self->rowcount == 0) { + Dprintf("skipping named cursor close because not existing"); + goto close; + } + } + EXC_IF_NO_MARK(self); - PyOS_snprintf(buffer, 127, "CLOSE %s", self->qname); - if (pq_execute(self, buffer, 0, 0, 1) == -1) return NULL; + PyOS_snprintf(buffer, bufsize, "CLOSE %s", self->qname); + if (pq_execute(self, buffer, 0, 0, 1) == -1) { goto exit; } } close: self->closed = 1; Dprintf("psyco_curs_close: cursor at %p closed", self); + rv = Py_None; + Py_INCREF(rv); + exit: - Py_RETURN_NONE; + PyMem_Free(lname); + return rv; } diff --git a/tests/test_cursor.py b/tests/test_cursor.py index b48fe7f..b0a3789 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -441,6 +441,19 @@ class CursorTests(ConnectingTestCase): cur = self.conn.cursor('test') cur.close() + @skip_before_postgres(8, 2) + def test_stolen_named_cursor_close(self): + cur1 = self.conn.cursor() + cur1.execute("DECLARE test CURSOR WITHOUT HOLD " + " FOR SELECT generate_series(1,7)") + cur2 = self.conn.cursor('test') + cur2.close() + + cur1.execute("DECLARE test CURSOR WITHOUT HOLD " + " FOR SELECT generate_series(1,7)") + cur2 = self.conn.cursor('test') + cur2.close() + @skip_before_postgres(8, 0) def test_scroll(self): cur = self.conn.cursor() -- cgit v1.2.1 From 466efe4461e5bb5f635f0121734d6c2ae520f459 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 24 Jul 2018 19:02:13 +0100 Subject: Bump tests for selective closure of named cursor to pg 8.2 Previous versions don't support the features as they don't have the pg_cursors view. But they are too old to care. --- tests/test_cursor.py | 2 +- tests/test_with.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index b0a3789..af44f9e 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -436,7 +436,7 @@ class CursorTests(ConnectingTestCase): self.assertEqual([(2,), (3,), (4,)], cur2.fetchmany(3)) self.assertEqual([(5,), (6,), (7,)], cur2.fetchall()) - @skip_before_postgres(8, 0) + @skip_before_postgres(8, 2) def test_named_noop_close(self): cur = self.conn.cursor('test') cur.close() diff --git a/tests/test_with.py b/tests/test_with.py index f26f8f9..1e4c518 100755 --- a/tests/test_with.py +++ b/tests/test_with.py @@ -215,7 +215,7 @@ class WithCursorTestCase(WithTestCase): else: self.fail("where is my exception?") - @skip_before_postgres(8, 0) + @skip_before_postgres(8, 2) def test_named_with_noop(self): with self.conn.cursor('named') as cur: pass -- cgit v1.2.1 From 97a4fb92c621416ac16738c1915efe0a1051faa7 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 24 Jul 2018 19:17:07 +0100 Subject: Fixed compile error on windows Because const int + 1 is not const, right??? Also fixed other occurrences of magic numbers and failed DRY around PyOS_snprintf() calls. --- psycopg/cursor_type.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 14647a4..5920c12 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -61,8 +61,7 @@ psyco_curs_close(cursorObject *self) } if (self->qname != NULL) { - const int bufsize = 255; - char buffer[bufsize + 1]; + char buffer[256]; PGTransactionStatusType status; if (self->conn) { @@ -91,7 +90,7 @@ psyco_curs_close(cursorObject *self) self->conn, self->name, -1, NULL, NULL))) { goto exit; } - PyOS_snprintf(buffer, bufsize, + PyOS_snprintf(buffer, sizeof(buffer), "SELECT 1 FROM pg_catalog.pg_cursors where name = %s", lname); if (pq_execute(self, buffer, 0, 0, 1) == -1) { goto exit; } @@ -103,7 +102,7 @@ psyco_curs_close(cursorObject *self) } EXC_IF_NO_MARK(self); - PyOS_snprintf(buffer, bufsize, "CLOSE %s", self->qname); + PyOS_snprintf(buffer, sizeof(buffer), "CLOSE %s", self->qname); if (pq_execute(self, buffer, 0, 0, 1) == -1) { goto exit; } } @@ -771,7 +770,7 @@ psyco_curs_fetchone(cursorObject *self) EXC_IF_NO_MARK(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->qname); + PyOS_snprintf(buffer, sizeof(buffer), "FETCH FORWARD 1 FROM %s", self->qname); if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -820,7 +819,7 @@ psyco_curs_next_named(cursorObject *self) if (self->row >= self->rowcount) { char buffer[128]; - PyOS_snprintf(buffer, 127, "FETCH FORWARD %ld FROM %s", + PyOS_snprintf(buffer, sizeof(buffer), "FETCH FORWARD %ld FROM %s", self->itersize, self->qname); if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; @@ -889,7 +888,7 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) EXC_IF_NO_MARK(self); EXC_IF_ASYNC_IN_PROGRESS(self, fetchmany); EXC_IF_TPC_PREPARED(self->conn, fetchone); - PyOS_snprintf(buffer, 127, "FETCH FORWARD %d FROM %s", + PyOS_snprintf(buffer, sizeof(buffer), "FETCH FORWARD %d FROM %s", (int)size, self->qname); if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) { goto exit; } if (_psyco_curs_prefetch(self) < 0) { goto exit; } @@ -965,7 +964,7 @@ psyco_curs_fetchall(cursorObject *self) EXC_IF_NO_MARK(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->qname); + PyOS_snprintf(buffer, sizeof(buffer), "FETCH FORWARD ALL FROM %s", self->qname); if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) { goto exit; } if (_psyco_curs_prefetch(self) < 0) { goto exit; } } @@ -1262,11 +1261,11 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) EXC_IF_TPC_PREPARED(self->conn, scroll); if (strcmp(mode, "absolute") == 0) { - PyOS_snprintf(buffer, 127, "MOVE ABSOLUTE %d FROM %s", + PyOS_snprintf(buffer, sizeof(buffer), "MOVE ABSOLUTE %d FROM %s", value, self->qname); } else { - PyOS_snprintf(buffer, 127, "MOVE %d FROM %s", value, self->qname); + PyOS_snprintf(buffer, sizeof(buffer), "MOVE %d FROM %s", value, self->qname); } if (pq_execute(self, buffer, 0, 0, self->withhold) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; -- cgit v1.2.1