diff --git a/NEWS b/NEWS index 67883d74..6ffa66a9 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,7 @@ New features: adapter is deprecated (:tickets:`#317, #343, #387`). - Added `~psycopg2.extensions.quote_ident()` function (:ticket:`#359`). - Added `~connection.get_dsn_parameters()` connection method (:ticket:`#364`). +- `~cursor.callproc()` now accepts a dictionary of parameters (:ticket:`#381`). Other changes: diff --git a/doc/src/cursor.rst b/doc/src/cursor.rst index 45e62781..aee6b465 100644 --- a/doc/src/cursor.rst +++ b/doc/src/cursor.rst @@ -201,13 +201,19 @@ The ``cursor`` class Call a stored database procedure with the given name. The sequence of parameters must contain one entry for each argument that the procedure - expects. The result of the call is returned as modified copy of the - input sequence. Input parameters are left untouched, output and - input/output parameters replaced with possibly new values. - - The procedure may also provide a result set as output. This must then - be made available through the standard |fetch*|_ methods. + expects. Overloaded procedures are supported. Named parameters can be + used by supplying the parameters as a dictionary. + This function is, at present, not DBAPI-compliant. The return value is + supposed to consist of the sequence of parameters with modified output + and input/output parameters. In future versions, the DBAPI-compliant + return value may be implemented, but for now the function returns None. + + The procedure may provide a result set as output. This is then made + available through the standard |fetch*|_ methods. + + .. versionchanged:: 2.7 + added support for named arguments. .. method:: mogrify(operation [, parameters]) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 6b66217c..baa5b8f7 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1025,10 +1025,17 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *operation = NULL; PyObject *res = NULL; - if (!PyArg_ParseTuple(args, "s#|O", - &procname, &procname_len, ¶meters - )) - { goto exit; } + int using_dict; + PyObject *pname = NULL; + PyObject *pnames = NULL; + PyObject *pvals = NULL; + char *cpname = NULL; + char **scpnames = NULL; + + if (!PyArg_ParseTuple(args, "s#|O", &procname, &procname_len, + ¶meters)) { + goto exit; + } EXC_IF_CURS_CLOSED(self); EXC_IF_ASYNC_IN_PROGRESS(self, callproc); @@ -1036,7 +1043,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (self->name != NULL) { psyco_set_error(ProgrammingError, self, - "can't call .callproc() on named cursors"); + "can't call .callproc() on named cursors"); goto exit; } @@ -1044,31 +1051,108 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (-1 == (nparameters = PyObject_Length(parameters))) { goto exit; } } - /* allocate some memory, build the SQL and create a PyString from it */ - sl = procname_len + 17 + nparameters*3 - (nparameters ? 1 : 0); - sql = (char*)PyMem_Malloc(sl); - if (sql == NULL) { - PyErr_NoMemory(); + using_dict = nparameters > 0 && PyDict_Check(parameters); + + /* a Dict is complicated; the parameter names go into the query */ + if (using_dict) { + if (!(pnames = PyDict_Keys(parameters))) { goto exit; } + if (!(pvals = PyDict_Values(parameters))) { goto exit; } + + sl = procname_len + 17 + nparameters * 5 - (nparameters ? 1 : 0); + + if (!(scpnames = PyMem_New(char *, nparameters))) { + PyErr_NoMemory(); + goto exit; + } + + memset(scpnames, 0, sizeof(char *) * nparameters); + + /* each parameter has to be processed; it's a few steps. */ + for (i = 0; i < nparameters; i++) { + /* all errors are RuntimeErrors as they should never occur */ + + if (!(pname = PyList_GetItem(pnames, i))) { goto exit; } + Py_INCREF(pname); /* was borrowed */ + + /* this also makes a check for keys being strings */ + if (!(pname = psycopg_ensure_bytes(pname))) { goto exit; } + if (!(cpname = Bytes_AsString(pname))) { goto exit; } + + if (!(scpnames[i] = psycopg_escape_identifier( + self->conn, cpname, 0))) { + Py_CLEAR(pname); + goto exit; + } + + Py_CLEAR(pname); + + sl += strlen(scpnames[i]); + } + + if (!(sql = (char*)PyMem_Malloc(sl))) { + PyErr_NoMemory(); + goto exit; + } + + sprintf(sql, "SELECT * FROM %s(", procname); + for (i = 0; i < nparameters; i++) { + strcat(sql, scpnames[i]); + strcat(sql, ":=%s,"); + } + sql[sl-2] = ')'; + sql[sl-1] = '\0'; + } + + /* a list (or None, or empty data structure) is a little bit simpler */ + else { + Py_INCREF(parameters); + pvals = parameters; + + sl = procname_len + 17 + nparameters * 3 - (nparameters ? 1 : 0); + + sql = (char*)PyMem_Malloc(sl); + if (sql == NULL) { + PyErr_NoMemory(); + goto exit; + } + + sprintf(sql, "SELECT * FROM %s(", procname); + for (i = 0; i < nparameters; i++) { + strcat(sql, "%s,"); + } + sql[sl-2] = ')'; + sql[sl-1] = '\0'; + } + + if (!(operation = Bytes_FromString(sql))) { goto exit; } - sprintf(sql, "SELECT * FROM %s(", procname); - for(i=0; iconn->async, 0)) { - Py_INCREF(parameters); - res = parameters; + if (0 <= _psyco_curs_execute( + self, operation, pvals, self->conn->async, 0)) { + /* The dict case is outside DBAPI scope anyway, so simply return None */ + if (using_dict) { + res = Py_None; + } + else { + res = pvals; + } + Py_INCREF(res); } exit: + if (scpnames != NULL) { + for (i = 0; i < nparameters; i++) { + if (scpnames[i] != NULL) { + PQfreemem(scpnames[i]); + } + } + } + PyMem_Del(scpnames); + Py_XDECREF(pname); + Py_XDECREF(pnames); Py_XDECREF(operation); + Py_XDECREF(pvals); PyMem_Free((void*)sql); return res; } diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 012df6b3..bf7d908a 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -62,7 +62,6 @@ HIDDEN PyObject *pyDateTimeModuleP = NULL; HIDDEN PyObject *psycoEncodings = NULL; - #ifdef PSYCOPG_DEBUG HIDDEN int psycopg_debug_enabled = 0; #endif @@ -191,9 +190,8 @@ psyco_quote_ident(PyObject *self, PyObject *args, PyObject *kwargs) str = Bytes_AS_STRING(ident); - quoted = PQescapeIdentifier(conn->pgconn, str, strlen(str)); + quoted = psycopg_escape_identifier(conn, str, strlen(str)); if (!quoted) { - PyErr_NoMemory(); goto exit; } result = conn_text_from_chars(conn, quoted); diff --git a/psycopg/utils.c b/psycopg/utils.c index c518cc57..bc6f7bec 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -123,6 +123,7 @@ exit: return rv; } + /* Duplicate a string. * * Allocate a new buffer on the Python heap containing the new string. diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 4aae6b2f..fc924c4b 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -498,6 +498,48 @@ class CursorTests(ConnectingTestCase): cur = self.conn.cursor() self.assertRaises(TypeError, cur.callproc, 'lower', 42) + # It would be inappropriate to test callproc's named parameters in the + # DBAPI2.0 test section because they are a psycopg2 extension. + @skip_before_postgres(9, 0) + def test_callproc_dict(self): + # This parameter name tests for injection and quote escaping + paramname = ''' + Robert'); drop table "students" -- + '''.strip() + escaped_paramname = '"%s"' % paramname.replace('"', '""') + procname = 'pg_temp.randall' + + cur = self.conn.cursor() + + # Set up the temporary function + cur.execute(''' + CREATE FUNCTION %s(%s INT) + RETURNS INT AS + 'SELECT $1 * $1' + LANGUAGE SQL + ''' % (procname, escaped_paramname)); + + # Make sure callproc works right + cur.callproc(procname, { paramname: 2 }) + self.assertEquals(cur.fetchone()[0], 4) + + # Make sure callproc fails right + failing_cases = [ + ({ paramname: 2, 'foo': 'bar' }, psycopg2.ProgrammingError), + ({ paramname: '2' }, psycopg2.ProgrammingError), + ({ paramname: 'two' }, psycopg2.ProgrammingError), + ({ u'bj\xc3rn': 2 }, psycopg2.ProgrammingError), + ({ 3: 2 }, TypeError), + ({ self: 2 }, TypeError), + ] + for parameter_sequence, exception in failing_cases: + self.assertRaises(exception, cur.callproc, procname, parameter_sequence) + self.conn.rollback() + + def test_callproc_badparam(self): + cur = self.conn.cursor() + self.assertRaises(TypeError, cur.callproc, 'lower', 42) + def test_suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/tests/test_psycopg2_dbapi20.py b/tests/test_psycopg2_dbapi20.py index 80473b70..c780d506 100755 --- a/tests/test_psycopg2_dbapi20.py +++ b/tests/test_psycopg2_dbapi20.py @@ -36,7 +36,31 @@ class Psycopg2Tests(dbapi20.DatabaseAPI20Test): connect_args = () connect_kw_args = {'dsn': dsn} - lower_func = 'lower' # For stored procedure test + lower_func = 'lower' # For stored procedure test + + def test_callproc(self): + # Until DBAPI 2.0 compliance, callproc should return None or it's just + # misleading. Therefore, we will skip the return value test for + # callproc and only perform the fetch test. + # + # For what it's worth, the DBAPI2.0 test_callproc doesn't actually + # test for DBAPI2.0 compliance! It doesn't check for modified OUT and + # IN/OUT parameters in the return values! + con = self._connect() + try: + cur = con.cursor() + if self.lower_func and hasattr(cur,'callproc'): + cur.callproc(self.lower_func,('FOO',)) + r = cur.fetchall() + self.assertEqual(len(r),1,'callproc produced no result set') + self.assertEqual(len(r[0]),1, + 'callproc produced invalid result set' + ) + self.assertEqual(r[0][0],'foo', + 'callproc produced invalid results' + ) + finally: + con.close() def test_setoutputsize(self): # psycopg2's setoutputsize() is a no-op