Merge branch 'cursor-args-fix' into MAINT_2_4

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
 ---------------------------
 

lib/extras.py

@@ -103,11 +103,9 @@ 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):
+        kwargs.setdefault('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 +194,9 @@ 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):
+        kwargs.setdefault('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 +250,7 @@ class RealDictRow(dict):
 class NamedTupleConnection(_connection):
     """A connection that uses `NamedTupleCursor` automatically."""
     def cursor(self, *args, **kwargs):
-        kwargs['cursor_factory'] = NamedTupleCursor
+        kwargs.setdefault('cursor_factory', NamedTupleCursor)
         return _connection.cursor(self, *args, **kwargs)
 
 class NamedTupleCursor(_cursor):
@@ -349,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.
 
@@ -358,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."""
@@ -390,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`.
     """
@@ -415,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)

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 = "

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;
@@ -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 *

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);")

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()