From c63891af8d435a6f37fc41128ba2dfa74a7a1f64 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 11 Apr 2012 17:26:11 +0100 Subject: [PATCH 1/6] Fixed bad error return code from cursor's init --- psycopg/cursor_type.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index bcbefc5f..6289f671 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1724,7 +1724,7 @@ cursor_setup(cursorObject *self, connectionObject *conn, const char *name) if (name) { if (!(self->name = psycopg_escape_identifier_easy(name, 0))) { - return 1; + return -1; } } @@ -1733,7 +1733,7 @@ cursor_setup(cursorObject *self, connectionObject *conn, const char *name) (PyObject *)&connectionType) == 0) { PyErr_SetString(PyExc_TypeError, "argument 1 must be subclass of psycopg2._psycopg.connection"); - return 1; + return -1; } */ Py_INCREF(conn); self->conn = conn; From 27421f1e41dd453c986bf3e746ed8f64abc440cc Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 11 Apr 2012 17:32:10 +0100 Subject: [PATCH 2/6] Name can be passed as None to cursor() Makes invocation from subclasses and generic code easier. Code simplified by using default values for keyword arguments and avoiding needless conversions back and forth between Python and C strings. Also added connection type check to cursor's init. --- psycopg/connection_type.c | 43 ++++++++++++++++++--------------------- psycopg/cursor_type.c | 21 +++++++++++++++---- tests/test_cursor.py | 4 ++++ 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 4e5b6170..c4b88b3e 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -42,7 +42,7 @@ /* cursor method - allocate a new cursor */ #define psyco_conn_cursor_doc \ -"cursor(name=None, cursor_factory=extensions.cursor, withhold=None) -- new cursor\n\n" \ +"cursor(name=None, cursor_factory=extensions.cursor, withhold=False) -- new cursor\n\n" \ "Return a new cursor.\n\nThe ``cursor_factory`` argument can be used to\n" \ "create non-standard cursors by passing a class different from the\n" \ "default. Note that the new class *should* be a sub-class of\n" \ @@ -50,26 +50,26 @@ ":rtype: `extensions.cursor`" static PyObject * -psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *keywds) +psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *kwargs) { - const char *name = NULL; - PyObject *obj, *factory = NULL, *withhold = NULL; + PyObject *obj; + PyObject *name = Py_None; + PyObject *factory = (PyObject *)&cursorType; + PyObject *withhold = Py_False; static char *kwlist[] = {"name", "cursor_factory", "withhold", NULL}; - if (!PyArg_ParseTupleAndKeywords(args, keywds, "|sOO", kwlist, + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOO", kwlist, &name, &factory, &withhold)) { return NULL; } - if (withhold != NULL) { - if (PyObject_IsTrue(withhold) && name == NULL) { - PyErr_SetString(ProgrammingError, - "'withhold=True can be specified only for named cursors"); - return NULL; - } + if (PyObject_IsTrue(withhold) && (name == Py_None)) { + PyErr_SetString(ProgrammingError, + "'withhold=True can be specified only for named cursors"); + return NULL; } - + EXC_IF_CONN_CLOSED(self); if (self->status != CONN_STATUS_READY && @@ -80,31 +80,28 @@ psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *keywds) return NULL; } - if (name != NULL && self->async == 1) { + if (name != Py_None && self->async == 1) { PyErr_SetString(ProgrammingError, "asynchronous connections " "cannot produce named cursors"); return NULL; } - Dprintf("psyco_conn_cursor: new cursor for connection at %p", self); - Dprintf("psyco_conn_cursor: parameters: name = %s", name); + Dprintf("psyco_conn_cursor: new %s cursor for connection at %p", + (name == Py_None ? "unnamed" : "named"), self); - if (factory == NULL) factory = (PyObject *)&cursorType; - if (name) - obj = PyObject_CallFunction(factory, "Os", self, name); - else - obj = PyObject_CallFunctionObjArgs(factory, self, NULL); + if (!(obj = PyObject_CallFunctionObjArgs(factory, self, name, NULL))) { + return NULL; + } - if (obj == NULL) return NULL; if (PyObject_IsInstance(obj, (PyObject *)&cursorType) == 0) { PyErr_SetString(PyExc_TypeError, "cursor factory must be subclass of psycopg2._psycopg.cursor"); Py_DECREF(obj); return NULL; } - - if (withhold != NULL && PyObject_IsTrue(withhold)) + + if (PyObject_IsTrue(withhold)) ((cursorObject*)obj)->withhold = 1; Dprintf("psyco_conn_cursor: new cursor at %p: refcnt = " diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 6289f671..30126e41 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1808,15 +1808,28 @@ cursor_dealloc(PyObject* obj) } static int -cursor_init(PyObject *obj, PyObject *args, PyObject *kwds) +cursor_init(PyObject *obj, PyObject *args, PyObject *kwargs) { - const char *name = NULL; PyObject *conn; + PyObject *name = Py_None; + const char *cname; - if (!PyArg_ParseTuple(args, "O|s", &conn, &name)) + static char *kwlist[] = {"conn", "name", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O!|O", kwlist, + &connectionType, &conn, &name)) { return -1; + } - return cursor_setup((cursorObject *)obj, (connectionObject *)conn, name); + if (name == Py_None) { + cname = NULL; + } else { + if (!(cname = Bytes_AsString(name))) { + return -1; + } + } + + return cursor_setup((cursorObject *)obj, (connectionObject *)conn, cname); } static PyObject * diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 051c9f27..b1c5c5c3 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -165,6 +165,10 @@ class CursorTests(unittest.TestCase): del curs self.assert_(w() is None) + def test_null_name(self): + curs = self.conn.cursor(None) + self.assertEqual(curs.name, None) + def test_invalid_name(self): curs = self.conn.cursor() curs.execute("create temp table invname (data int);") From 095cce5605cc3bf460298c954ab6a90455e2a81a Mon Sep 17 00:00:00 2001 From: Corry Haines Date: Sun, 25 Mar 2012 20:49:54 -0700 Subject: [PATCH 3/6] Allow user to override connection factory cursors Prior to this change, using a extras.connection_factory would not allow any other cursor to be used on that connection. It was set in stone. This change allows all cursor options to pass through and override the connection factory behaviors. This allows a connection_factory to be dropped into existing code with no disruption. This change also standardizes the extras.connection_factories to have the same behavior and all pass through *args and **kwargs. --- lib/extras.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/extras.py b/lib/extras.py index 2e3974b6..b1d4d9e4 100644 --- a/lib/extras.py +++ b/lib/extras.py @@ -103,11 +103,10 @@ class DictCursorBase(_cursor): class DictConnection(_connection): """A connection that uses `DictCursor` automatically.""" - def cursor(self, name=None): - if name is None: - return _connection.cursor(self, cursor_factory=DictCursor) - else: - return _connection.cursor(self, name, cursor_factory=DictCursor) + def cursor(self, *args, **kwargs): + if 'cursor_factory' not in kwargs: + kwargs['cursor_factory'] = DictCursor + return _connection.cursor(self, *args, **kwargs) class DictCursor(DictCursorBase): """A cursor that keeps a list of column name -> index mappings.""" @@ -196,11 +195,10 @@ class DictRow(list): class RealDictConnection(_connection): """A connection that uses `RealDictCursor` automatically.""" - def cursor(self, name=None): - if name is None: - return _connection.cursor(self, cursor_factory=RealDictCursor) - else: - return _connection.cursor(self, name, cursor_factory=RealDictCursor) + def cursor(self, *args, **kwargs): + if 'cursor_factory' not in kwargs: + kwargs['cursor_factory'] = RealDictCursor + return _connection.cursor(self, *args, **kwargs) class RealDictCursor(DictCursorBase): """A cursor that uses a real dict as the base type for rows. @@ -254,7 +252,8 @@ class RealDictRow(dict): class NamedTupleConnection(_connection): """A connection that uses `NamedTupleCursor` automatically.""" def cursor(self, *args, **kwargs): - kwargs['cursor_factory'] = NamedTupleCursor + if 'cursor_factory' not in kwargs: + kwargs['cursor_factory'] = NamedTupleCursor return _connection.cursor(self, *args, **kwargs) class NamedTupleCursor(_cursor): From c86ca7687fe66f01a1b645ebacad735f1b693a56 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 11 Apr 2012 17:59:16 +0100 Subject: [PATCH 4/6] Fixed cursor() arguments propagation to other connection classes --- lib/extras.py | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/lib/extras.py b/lib/extras.py index b1d4d9e4..1560edd3 100644 --- a/lib/extras.py +++ b/lib/extras.py @@ -104,8 +104,7 @@ class DictCursorBase(_cursor): class DictConnection(_connection): """A connection that uses `DictCursor` automatically.""" def cursor(self, *args, **kwargs): - if 'cursor_factory' not in kwargs: - kwargs['cursor_factory'] = DictCursor + kwargs.setdefault('cursor_factory', DictCursor) return _connection.cursor(self, *args, **kwargs) class DictCursor(DictCursorBase): @@ -196,8 +195,7 @@ class DictRow(list): class RealDictConnection(_connection): """A connection that uses `RealDictCursor` automatically.""" def cursor(self, *args, **kwargs): - if 'cursor_factory' not in kwargs: - kwargs['cursor_factory'] = RealDictCursor + kwargs.setdefault('cursor_factory', RealDictCursor) return _connection.cursor(self, *args, **kwargs) class RealDictCursor(DictCursorBase): @@ -252,8 +250,7 @@ class RealDictRow(dict): class NamedTupleConnection(_connection): """A connection that uses `NamedTupleCursor` automatically.""" def cursor(self, *args, **kwargs): - if 'cursor_factory' not in kwargs: - kwargs['cursor_factory'] = NamedTupleCursor + kwargs.setdefault('cursor_factory', NamedTupleCursor) return _connection.cursor(self, *args, **kwargs) class NamedTupleCursor(_cursor): @@ -348,7 +345,7 @@ class LoggingConnection(_connection): self.log = self._logtologger else: self.log = self._logtofile - + def filter(self, msg, curs): """Filter the query before logging it. @@ -357,26 +354,24 @@ class LoggingConnection(_connection): just does nothing. """ return msg - + def _logtofile(self, msg, curs): msg = self.filter(msg, curs) if msg: self._logobj.write(msg + os.linesep) - + def _logtologger(self, msg, curs): msg = self.filter(msg, curs) if msg: self._logobj.debug(msg) - + def _check(self): if not hasattr(self, '_logobj'): raise self.ProgrammingError( "LoggingConnection object has not been initialize()d") - - def cursor(self, name=None): + + def cursor(self, *args, **kwargs): self._check() - if name is None: - return _connection.cursor(self, cursor_factory=LoggingCursor) - else: - return _connection.cursor(self, name, cursor_factory=LoggingCursor) + kwargs.setdefault('cursor_factory', LoggingCursor) + return _connection.cursor(self, *args, **kwargs) class LoggingCursor(_cursor): """A cursor that logs queries using its connection logging facilities.""" @@ -389,19 +384,19 @@ class LoggingCursor(_cursor): def callproc(self, procname, vars=None): try: - return _cursor.callproc(self, procname, vars) + return _cursor.callproc(self, procname, vars) finally: self.connection.log(self.query, self) class MinTimeLoggingConnection(LoggingConnection): """A connection that logs queries based on execution time. - + This is just an example of how to sub-class `LoggingConnection` to provide some extra filtering for the logged queries. Both the `inizialize()` and `filter()` methods are overwritten to make sure that only queries executing for more than ``mintime`` ms are logged. - + Note that this connection uses the specialized cursor `MinTimeLoggingCursor`. """ @@ -414,20 +409,17 @@ class MinTimeLoggingConnection(LoggingConnection): if t > self._mintime: return msg + os.linesep + " (execution time: %d ms)" % t - def cursor(self, name=None): - self._check() - if name is None: - return _connection.cursor(self, cursor_factory=MinTimeLoggingCursor) - else: - return _connection.cursor(self, name, cursor_factory=MinTimeLoggingCursor) - + def cursor(self, *args, **kwargs): + kwargs.setdefault('cursor_factory', MinTimeLoggingCursor) + return LoggingConnection.cursor(self, *args, **kwargs) + class MinTimeLoggingCursor(LoggingCursor): """The cursor sub-class companion to `MinTimeLoggingConnection`.""" def execute(self, query, vars=None): self.timestamp = time.time() return LoggingCursor.execute(self, query, vars) - + def callproc(self, procname, vars=None): self.timestamp = time.time() return LoggingCursor.execute(self, procname, vars) From 365a1b20a7cd5b6b5a58bb35250d5c29767f06b9 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 11 Apr 2012 18:00:18 +0100 Subject: [PATCH 5/6] Added tests to verify cursor() arguments propagation --- tests/test_extras_dictcursor.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_extras_dictcursor.py b/tests/test_extras_dictcursor.py index dd746379..34e571d3 100755 --- a/tests/test_extras_dictcursor.py +++ b/tests/test_extras_dictcursor.py @@ -35,6 +35,16 @@ class ExtrasDictCursorTests(unittest.TestCase): def tearDown(self): self.conn.close() + def testDictConnCursorArgs(self): + self.conn.close() + self.conn = psycopg2.connect(dsn, connection_factory=psycopg2.extras.DictConnection) + cur = self.conn.cursor() + self.assert_(isinstance(cur, psycopg2.extras.DictCursor)) + self.assertEqual(cur.name, None) + # overridable + cur = self.conn.cursor('foo', cursor_factory=psycopg2.extras.NamedTupleCursor) + self.assertEqual(cur.name, 'foo') + self.assert_(isinstance(cur, psycopg2.extras.NamedTupleCursor)) def testDictCursorWithPlainCursorFetchOne(self): self._testWithPlainCursor(lambda curs: curs.fetchone()) @@ -219,6 +229,12 @@ class NamedTupleCursorTest(unittest.TestCase): if self.conn is not None: self.conn.close() + @skip_if_no_namedtuple + def test_cursor_args(self): + cur = self.conn.cursor('foo', cursor_factory=psycopg2.extras.DictCursor) + self.assertEqual(cur.name, 'foo') + self.assert_(isinstance(cur, psycopg2.extras.DictCursor)) + @skip_if_no_namedtuple def test_fetchone(self): curs = self.conn.cursor() From 73df259f7bfd8492990cf6b75608b4cf271b50a7 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 11 Apr 2012 18:11:04 +0100 Subject: [PATCH 6/6] Added news entry about cursor() cleanup --- NEWS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS b/NEWS index 71ef7336..880278be 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,11 @@ +What's new in psycopg 2.4.6 +--------------------------- + + - Fixed 'cursor()' arguments propagation in connection subclasses + and overriding of the 'cursor_factory' argument. Thanks to + Corry Haines for the report and the initial patch (ticket #105). + + What's new in psycopg 2.4.5 ---------------------------