From 23d279945ffb3eae3bea69856b20e8271108ebd0 Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Sat, 24 May 2014 23:47:09 -0400 Subject: [PATCH 01/41] cursor.callproc now also accepts dict for PostgreSQL 9+ "named notation" --- psycopg/cursor_type.c | 56 ++++++++++++++++++++++++++++++++++--------- psycopg/python.h | 2 ++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index cd8d5ca3..7992ce4b 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1024,6 +1024,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *parameters = Py_None; PyObject *operation = NULL; PyObject *res = NULL; + PyObject *parameter_names = NULL; if (!PyArg_ParseTuple(args, "s#|O", &procname, &procname_len, ¶meters @@ -1045,19 +1046,52 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) } /* 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(); - goto exit; - } - sprintf(sql, "SELECT * FROM %s(", procname); - for(i=0; i 0 && PyDict_Check(parameters)) { + /* for a dict, we put the parameter names into the SQL */ + parameter_names = PyDict_Keys(parameters); + + /* first we need to figure out how much space we need for the SQL */ + sl = procname_len + 17 + nparameters*5 - (nparameters ? 1 : 0); + for(i=0; i 2 From 1205bf9c2b123a0fbc81352af330ac2344251c62 Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Sun, 25 May 2014 04:03:07 -0400 Subject: [PATCH 02/41] callproc using a dict now has a type check to make sure the keys are strings. --- psycopg/cursor_type.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 7992ce4b..16339971 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1024,6 +1024,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *parameters = Py_None; PyObject *operation = NULL; PyObject *res = NULL; + PyObject *parameter_name = NULL; PyObject *parameter_names = NULL; if (!PyArg_ParseTuple(args, "s#|O", @@ -1054,7 +1055,12 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* first we need to figure out how much space we need for the SQL */ sl = procname_len + 17 + nparameters*5 - (nparameters ? 1 : 0); for(i=0; i Date: Wed, 28 May 2014 01:05:17 -0400 Subject: [PATCH 03/41] callproc using a dict now uses connection encoding and sanitizes parameter names --- psycopg/cursor_type.c | 57 ++++++++++++++++++++++++++++++++++--------- psycopg/python.h | 2 -- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 16339971..113e0a54 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1025,6 +1025,10 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *operation = NULL; PyObject *res = NULL; PyObject *parameter_name = NULL; + PyObject *parameter_name_bytes = NULL; + char *parameter_name_cstr = NULL; + char *parameter_name_cstr_sanitized = NULL; + char **parameter_name_cstr_sanitized_CACHE = NULL; PyObject *parameter_names = NULL; if (!PyArg_ParseTuple(args, "s#|O", @@ -1048,40 +1052,71 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* allocate some memory, build the SQL and create a PyString from it */ + /* a dict requires special handling: we put the parameter names into the SQL */ if (nparameters > 0 && PyDict_Check(parameters)) { - /* for a dict, we put the parameter names into the SQL */ + parameter_names = PyDict_Keys(parameters); - /* first we need to figure out how much space we need for the SQL */ - sl = procname_len + 17 + nparameters*5 - (nparameters ? 1 : 0); + /* first we need to ensure the dict's keys are text */ for(i=0; iconn->pgconn, parameter_name_cstr, strlen(parameter_name_cstr)); + Py_DECREF(parameter_name); + + /* must add the length of the sanitized string to the length of the SQL string */ + sl += strlen(parameter_name_cstr_sanitized); + + parameter_name_cstr_sanitized_CACHE[i] = parameter_name_cstr_sanitized; + } + + Py_DECREF(parameter_names); + sql = (char*)PyMem_Malloc(sl); if (sql == NULL) { - PyErr_NoMemory(); - goto exit; + PyErr_NoMemory(); + for(i=0; i 2 From e9bb4a86f93836f8f6c1233c45507e58c159ef1f Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Wed, 28 May 2014 01:27:56 -0400 Subject: [PATCH 04/41] cursor.callproc: added a missing memory check --- psycopg/cursor_type.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 113e0a54..5f70ce61 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1072,6 +1072,13 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* we will throw the sanitized C strings into a cache to not redo the work later */ parameter_name_cstr_sanitized_CACHE = PyMem_New(char *, nparameters); + if (parameter_name_cstr_sanitized_CACHE == NULL) { + PyErr_NoMemory(); + PyMem_Del(parameter_name_cstr_sanitized_CACHE); + Py_DECREF(parameter_names); + goto exit; + } + for(i=0; i Date: Thu, 29 May 2014 04:31:43 -0400 Subject: [PATCH 05/41] callproc: now more compliant with local coding standards. --- psycopg/cursor_type.c | 192 ++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 90 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 5f70ce61..d6875a79 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1024,17 +1024,18 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *parameters = Py_None; PyObject *operation = NULL; PyObject *res = NULL; - PyObject *parameter_name = NULL; - PyObject *parameter_name_bytes = NULL; - char *parameter_name_cstr = NULL; - char *parameter_name_cstr_sanitized = NULL; - char **parameter_name_cstr_sanitized_CACHE = NULL; - PyObject *parameter_names = NULL; - if (!PyArg_ParseTuple(args, "s#|O", - &procname, &procname_len, ¶meters - )) - { goto exit; } + int using_dict; + PyObject *pname = NULL; + PyObject *bpname = NULL; + PyObject *pnames = 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); @@ -1042,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; } @@ -1050,106 +1051,117 @@ 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 */ + using_dict = nparameters > 0 && PyDict_Check(parameters); - /* a dict requires special handling: we put the parameter names into the SQL */ - if (nparameters > 0 && PyDict_Check(parameters)) { - - parameter_names = PyDict_Keys(parameters); - - /* first we need to ensure the dict's keys are text */ - for(i=0; iconn->pgconn, parameter_name_cstr, strlen(parameter_name_cstr)); - Py_DECREF(parameter_name); - - /* must add the length of the sanitized string to the length of the SQL string */ - sl += strlen(parameter_name_cstr_sanitized); - - parameter_name_cstr_sanitized_CACHE[i] = parameter_name_cstr_sanitized; - } - - Py_DECREF(parameter_names); - - sql = (char*)PyMem_Malloc(sl); - if (sql == NULL) { - PyErr_NoMemory(); - for(i=0; iconn->pgconn, cpname, + strlen(cpname)))) { + PyErr_SetString(PyExc_RuntimeError, + "libpq failed to escape identifier!"); + goto exit; + } + + sl += strlen(scpnames[i]); + } + + 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, scpnames[i]); + strcat(sql, ":=%s,"); + } + sql[sl-2] = ')'; + sql[sl-1] = '\0'; + + if (!(parameters = PyDict_Values(parameters))) { + PyErr_SetString(PyExc_RuntimeError, + "built-in 'values' failed on a Dict!"); + goto exit; + } } - /* a list (or None) is a little bit simpler */ + /* a list (or None, or empty data structure) is a little bit simpler */ else { - sl = procname_len + 17 + nparameters*3 - (nparameters ? 1 : 0); + sl = procname_len + 17 + nparameters * 3 - (nparameters ? 1 : 0); - sql = (char*)PyMem_Malloc(sl); - if (sql == NULL) { - PyErr_NoMemory(); - goto exit; - } + sql = (char*)PyMem_Malloc(sl); + if (sql == NULL) { + PyErr_NoMemory(); + goto exit; + } - sprintf(sql, "SELECT * FROM %s(", procname); - for(i=0; iconn->async, 0)) { - Py_INCREF(parameters); + self->conn->async, 0)) { + /* In the dict case, the parameters are already a new reference */ + if (!using_dict) { + Py_INCREF(parameters); + } res = parameters; } exit: + if (scpnames != NULL) { + for (i = 0; i < nparameters; i++) { + if (scpnames[i] != NULL) { + PQfreemem(scpnames[i]); + } + } + } + PyMem_Del(scpnames); + Py_XDECREF(pnames); Py_XDECREF(operation); PyMem_Free((void*)sql); return res; From 37a80e9de8d6df13d9e43e4106b62cf74308c5d9 Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Thu, 29 May 2014 05:09:09 -0400 Subject: [PATCH 06/41] callproc: checking for libpq 9.0+ on compile. yes: use PQescapeIdentifier. no: error --- psycopg/cursor_type.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index d6875a79..3dbdbdc4 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1026,11 +1026,13 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) PyObject *res = NULL; int using_dict; +#if PG_VERSION_HEX >= 0x090000 PyObject *pname = NULL; PyObject *bpname = NULL; PyObject *pnames = NULL; char *cpname = NULL; char **scpnames = NULL; +#endif if (!PyArg_ParseTuple(args, "s#|O", &procname, &procname_len, ¶meters)) { @@ -1055,6 +1057,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* A Dict is complicated. The parameter names go into the query */ if (using_dict) { +#if PG_VERSION_HEX >= 0x090000 if (!(pnames = PyDict_Keys(parameters))) { PyErr_SetString(PyExc_RuntimeError, "built-in 'keys' failed on a Dict!"); @@ -1119,6 +1122,11 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) "built-in 'values' failed on a Dict!"); goto exit; } +#else + PyErr_SetString(PyExc_NotImplementedError, + "named parameters require psycopg2 compiled against libpq 9.0+"); + goto exit; +#endif } /* a list (or None, or empty data structure) is a little bit simpler */ @@ -1153,6 +1161,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) } exit: +#if PG_VERSION_HEX >= 0x090000 if (scpnames != NULL) { for (i = 0; i < nparameters; i++) { if (scpnames[i] != NULL) { @@ -1162,6 +1171,7 @@ exit: } PyMem_Del(scpnames); Py_XDECREF(pnames); +#endif Py_XDECREF(operation); PyMem_Free((void*)sql); return res; From c205f140a00f030bb2b7da943aba02653454d633 Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Fri, 30 May 2014 20:34:19 -0400 Subject: [PATCH 07/41] callproc: tests, docs, and comment/error-reporting touchups. --- doc/src/cursor.rst | 17 ++++++++++------ psycopg/cursor_type.c | 36 +++++++++++++++++++++++---------- tests/test_cursor.py | 37 ++++++++++++++++++++++++++++++++++ tests/test_psycopg2_dbapi20.py | 23 +++++++++++++++++++++ 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/doc/src/cursor.rst b/doc/src/cursor.rst index 73bb5375..9df65865 100644 --- a/doc/src/cursor.rst +++ b/doc/src/cursor.rst @@ -201,12 +201,17 @@ 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 with a PostgreSQL 9.0+ client by supplying the sequence of + parameters as a Dict. + + 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. .. method:: mogrify(operation [, parameters]) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 3dbdbdc4..8214d359 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1028,6 +1028,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) int using_dict; #if PG_VERSION_HEX >= 0x090000 PyObject *pname = NULL; + PyObject *spname = NULL; PyObject *bpname = NULL; PyObject *pnames = NULL; char *cpname = NULL; @@ -1055,7 +1056,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) using_dict = nparameters > 0 && PyDict_Check(parameters); - /* A Dict is complicated. The parameter names go into the query */ + /* a Dict is complicated; the parameter names go into the query */ if (using_dict) { #if PG_VERSION_HEX >= 0x090000 if (!(pnames = PyDict_Keys(parameters))) { @@ -1073,33 +1074,46 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) memset(scpnames, 0, sizeof(char *) * nparameters); - /* Each parameter has to be processed. It's a few steps. */ + /* 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))) { PyErr_SetString(PyExc_RuntimeError, "built-in 'values' did not return List!"); goto exit; } - if (!(bpname = psycopg_ensure_bytes(pname))) { - PyErr_SetString(PyExc_TypeError, - "argument 2 must have only string keys if Dict"); + if (!(spname = PyObject_Str(pname))) { + PyErr_SetString(PyExc_RuntimeError, + "built-in 'str' failed!"); + goto exit; + } + + /* this is the only function here that returns a new reference */ + if (!(bpname = psycopg_ensure_bytes(spname))) { + PyErr_SetString(PyExc_RuntimeError, + "failed to get Bytes from text!"); goto exit; } if (!(cpname = Bytes_AsString(bpname))) { + Py_XDECREF(bpname); PyErr_SetString(PyExc_RuntimeError, - "failed to get Bytes from String!"); + "failed to get cstr from Bytes!"); goto exit; } if (!(scpnames[i] = PQescapeIdentifier(self->conn->pgconn, cpname, strlen(cpname)))) { + Py_XDECREF(bpname); PyErr_SetString(PyExc_RuntimeError, "libpq failed to escape identifier!"); goto exit; } + Py_XDECREF(bpname); + sl += strlen(scpnames[i]); } @@ -1153,11 +1167,11 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (0 <= _psyco_curs_execute(self, operation, parameters, self->conn->async, 0)) { - /* In the dict case, the parameters are already a new reference */ - if (!using_dict) { - Py_INCREF(parameters); - } - res = parameters; + if (using_dict) { + Py_DECREF(parameters); + } + /* return None from this until it's DBAPI compliant... */ + res = Py_None; } exit: diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 970cc37d..00d19dfb 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -490,6 +490,43 @@ 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), + ({ 'bjørn': 2 }, psycopg2.ProgrammingError), + ({ 3: 2 }, psycopg2.ProgrammingError), + ({ self: 2 }, psycopg2.ProgrammingError), + ] + for parameter_sequence, exception in failing_cases: + self.assertRaises(exception, cur.callproc, procname, parameter_sequence) + self.conn.rollback() def test_suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/tests/test_psycopg2_dbapi20.py b/tests/test_psycopg2_dbapi20.py index 744d3224..28ea6690 100755 --- a/tests/test_psycopg2_dbapi20.py +++ b/tests/test_psycopg2_dbapi20.py @@ -36,6 +36,29 @@ class Psycopg2Tests(dbapi20.DatabaseAPI20Test): connect_kw_args = {'dsn': dsn} 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 From 04ce14b251bbbd1a6b48f26e58135e5e53d4080d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 01:25:22 +0200 Subject: [PATCH 08/41] Avoid clobbering the exceptions raised by other calls --- psycopg/cursor_type.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 8214d359..835f278b 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1060,8 +1060,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (using_dict) { #if PG_VERSION_HEX >= 0x090000 if (!(pnames = PyDict_Keys(parameters))) { - PyErr_SetString(PyExc_RuntimeError, - "built-in 'keys' failed on a Dict!"); goto exit; } @@ -1079,36 +1077,26 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* all errors are RuntimeErrors as they should never occur */ if (!(pname = PyList_GetItem(pnames, i))) { - PyErr_SetString(PyExc_RuntimeError, - "built-in 'values' did not return List!"); goto exit; } if (!(spname = PyObject_Str(pname))) { - PyErr_SetString(PyExc_RuntimeError, - "built-in 'str' failed!"); goto exit; } /* this is the only function here that returns a new reference */ if (!(bpname = psycopg_ensure_bytes(spname))) { - PyErr_SetString(PyExc_RuntimeError, - "failed to get Bytes from text!"); goto exit; } if (!(cpname = Bytes_AsString(bpname))) { Py_XDECREF(bpname); - PyErr_SetString(PyExc_RuntimeError, - "failed to get cstr from Bytes!"); goto exit; } if (!(scpnames[i] = PQescapeIdentifier(self->conn->pgconn, cpname, strlen(cpname)))) { Py_XDECREF(bpname); - PyErr_SetString(PyExc_RuntimeError, - "libpq failed to escape identifier!"); goto exit; } @@ -1132,8 +1120,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) sql[sl-1] = '\0'; if (!(parameters = PyDict_Values(parameters))) { - PyErr_SetString(PyExc_RuntimeError, - "built-in 'values' failed on a Dict!"); goto exit; } #else From a3eed9c9f530f62b6d4c595b0f66ab16c5a4978d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 01:32:58 +0200 Subject: [PATCH 09/41] Added guard on params with no length on callproc --- 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 835f278b..effb2e66 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1058,7 +1058,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* a Dict is complicated; the parameter names go into the query */ if (using_dict) { -#if PG_VERSION_HEX >= 0x090000 +#if PG_VERSION_NUM >= 90000 if (!(pnames = PyDict_Keys(parameters))) { goto exit; } From d297976d6d01273745e8e1f18b435d84e84984fc Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 02:18:05 +0200 Subject: [PATCH 10/41] Raise TypeError if the dict in callproc param contains non-strings Check-and-conversion chain fixed and simplified. 'spname' was a reference leak. --- psycopg/cursor_type.c | 38 ++++++++++++-------------------------- tests/test_cursor.py | 6 +++--- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index effb2e66..b65b7d22 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1028,8 +1028,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) int using_dict; #if PG_VERSION_HEX >= 0x090000 PyObject *pname = NULL; - PyObject *spname = NULL; - PyObject *bpname = NULL; PyObject *pnames = NULL; char *cpname = NULL; char **scpnames = NULL; @@ -1076,37 +1074,24 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) for (i = 0; i < nparameters; i++) { /* all errors are RuntimeErrors as they should never occur */ - if (!(pname = PyList_GetItem(pnames, i))) { + 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] = PQescapeIdentifier( + self->conn->pgconn, cpname, strlen(cpname)))) { goto exit; } - if (!(spname = PyObject_Str(pname))) { - goto exit; - } - - /* this is the only function here that returns a new reference */ - if (!(bpname = psycopg_ensure_bytes(spname))) { - goto exit; - } - - if (!(cpname = Bytes_AsString(bpname))) { - Py_XDECREF(bpname); - goto exit; - } - - if (!(scpnames[i] = PQescapeIdentifier(self->conn->pgconn, cpname, - strlen(cpname)))) { - Py_XDECREF(bpname); - goto exit; - } - - Py_XDECREF(bpname); + Py_CLEAR(pname); sl += strlen(scpnames[i]); } - sql = (char*)PyMem_Malloc(sl); - if (sql == NULL) { + if (!(sql = (char*)PyMem_Malloc(sl))) { PyErr_NoMemory(); goto exit; } @@ -1170,6 +1155,7 @@ exit: } } PyMem_Del(scpnames); + Py_XDECREF(pname); Py_XDECREF(pnames); #endif Py_XDECREF(operation); diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 00d19dfb..00143ee9 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -520,9 +520,9 @@ class CursorTests(ConnectingTestCase): ({ paramname: 2, 'foo': 'bar' }, psycopg2.ProgrammingError), ({ paramname: '2' }, psycopg2.ProgrammingError), ({ paramname: 'two' }, psycopg2.ProgrammingError), - ({ 'bjørn': 2 }, psycopg2.ProgrammingError), - ({ 3: 2 }, psycopg2.ProgrammingError), - ({ self: 2 }, 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) From 7302f348bc7ec7121e04aae70eb8e7705b971334 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 02:32:53 +0200 Subject: [PATCH 11/41] Added test with objects without length as callproc param --- tests/test_cursor.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 00143ee9..552b29c0 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -528,6 +528,11 @@ class CursorTests(ConnectingTestCase): 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__) From 021f6d22ad5995a24439befdf5acde2590556cf4 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 12:44:04 +0200 Subject: [PATCH 12/41] More straightforward param refcount handling in callproc --- psycopg/cursor_type.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index b65b7d22..06a43f18 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1022,6 +1022,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) char *sql = NULL; Py_ssize_t procname_len, i, nparameters = 0, sl = 0; PyObject *parameters = Py_None; + PyObject *pvals = NULL; PyObject *operation = NULL; PyObject *res = NULL; @@ -1057,9 +1058,8 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* a Dict is complicated; the parameter names go into the query */ if (using_dict) { #if PG_VERSION_NUM >= 90000 - if (!(pnames = PyDict_Keys(parameters))) { - goto exit; - } + if (!(pnames = PyDict_Keys(parameters))) { goto exit; } + if (!(pvals = PyDict_Values(parameters))) { goto exit; } sl = procname_len + 17 + nparameters * 5 - (nparameters ? 1 : 0); @@ -1104,9 +1104,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) sql[sl-2] = ')'; sql[sl-1] = '\0'; - if (!(parameters = PyDict_Values(parameters))) { - goto exit; - } #else PyErr_SetString(PyExc_NotImplementedError, "named parameters require psycopg2 compiled against libpq 9.0+"); @@ -1116,6 +1113,9 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* 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); @@ -1136,11 +1136,9 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) goto exit; } - if (0 <= _psyco_curs_execute(self, operation, parameters, - self->conn->async, 0)) { - if (using_dict) { - Py_DECREF(parameters); - } + if (0 <= _psyco_curs_execute( + self, operation, pvals, self->conn->async, 0)) { + /* return None from this until it's DBAPI compliant... */ res = Py_None; } @@ -1159,6 +1157,7 @@ exit: Py_XDECREF(pnames); #endif Py_XDECREF(operation); + Py_XDECREF(pvals); PyMem_Free((void*)sql); return res; } From 4003b7c977fba9897da21446707671157a67e6ad Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 12:45:39 +0200 Subject: [PATCH 13/41] Fixed callproc return value refcount Temporary anyway: I want to go back returning a list (or dict). --- psycopg/cursor_type.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 06a43f18..d10bfb82 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1139,8 +1139,9 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (0 <= _psyco_curs_execute( self, operation, pvals, self->conn->async, 0)) { - /* return None from this until it's DBAPI compliant... */ - res = Py_None; + /* return None from this until it's DBAPI compliant... */ + Py_INCREF(Py_None); + res = Py_None; } exit: From 54e5349f531df58b08243f74c03ce13ab8708263 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 13:17:51 +0200 Subject: [PATCH 14/41] Set an exception in case of PQescapeIdentifier error Ifdeffed surface reduced. --- psycopg/cursor_type.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index d10bfb82..fd8fbbee 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1015,6 +1015,34 @@ exit: #define psyco_curs_callproc_doc \ "callproc(procname, parameters=None) -- Execute stored procedure." +/* Call PQescapeIdentifier. + * + * In case of error set a Python exception. + * + * TODO: this function can become more generic and go into utils + */ +static char * +_escape_identifier(PGconn *pgconn, const char *str, size_t length) +{ + char *rv = NULL; + +#if PG_VERSION_NUM >= 90000 + rv = PQescapeIdentifier(pgconn, str, length); + if (!rv) { + char *msg; + if (!(msg = PQerrorMessage(pgconn))) { + msg = "no message provided"; + } + PyErr_Format(InterfaceError, "failed to escape identifier: %s", msg); + } +#else + PyErr_Format(PyExc_NotImplementedError, + "named parameters require psycopg2 compiled against libpq 9.0+"); +#endif + + return rv; +} + static PyObject * psyco_curs_callproc(cursorObject *self, PyObject *args) { @@ -1022,17 +1050,15 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) char *sql = NULL; Py_ssize_t procname_len, i, nparameters = 0, sl = 0; PyObject *parameters = Py_None; - PyObject *pvals = NULL; PyObject *operation = NULL; PyObject *res = NULL; int using_dict; -#if PG_VERSION_HEX >= 0x090000 PyObject *pname = NULL; PyObject *pnames = NULL; + PyObject *pvals = NULL; char *cpname = NULL; char **scpnames = NULL; -#endif if (!PyArg_ParseTuple(args, "s#|O", &procname, &procname_len, ¶meters)) { @@ -1057,7 +1083,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) /* a Dict is complicated; the parameter names go into the query */ if (using_dict) { -#if PG_VERSION_NUM >= 90000 if (!(pnames = PyDict_Keys(parameters))) { goto exit; } if (!(pvals = PyDict_Values(parameters))) { goto exit; } @@ -1081,7 +1106,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (!(pname = psycopg_ensure_bytes(pname))) { goto exit; } if (!(cpname = Bytes_AsString(pname))) { goto exit; } - if (!(scpnames[i] = PQescapeIdentifier( + if (!(scpnames[i] = _escape_identifier( self->conn->pgconn, cpname, strlen(cpname)))) { goto exit; } @@ -1103,12 +1128,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) } sql[sl-2] = ')'; sql[sl-1] = '\0'; - -#else - PyErr_SetString(PyExc_NotImplementedError, - "named parameters require psycopg2 compiled against libpq 9.0+"); - goto exit; -#endif } /* a list (or None, or empty data structure) is a little bit simpler */ @@ -1145,7 +1164,6 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) } exit: -#if PG_VERSION_HEX >= 0x090000 if (scpnames != NULL) { for (i = 0; i < nparameters; i++) { if (scpnames[i] != NULL) { @@ -1156,7 +1174,6 @@ exit: PyMem_Del(scpnames); Py_XDECREF(pname); Py_XDECREF(pnames); -#endif Py_XDECREF(operation); Py_XDECREF(pvals); PyMem_Free((void*)sql); From 92109e4bbad9cdcd86dd9bf2f40bae5c26202a0d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 5 Jun 2014 13:41:45 +0200 Subject: [PATCH 15/41] Correctly handle an empty error message from PQescapeIdentifier --- psycopg/cursor_type.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index fd8fbbee..0a785c05 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1030,7 +1030,8 @@ _escape_identifier(PGconn *pgconn, const char *str, size_t length) rv = PQescapeIdentifier(pgconn, str, length); if (!rv) { char *msg; - if (!(msg = PQerrorMessage(pgconn))) { + msg = PQerrorMessage(pgconn); + if (!msg || !msg[0]) { msg = "no message provided"; } PyErr_Format(InterfaceError, "failed to escape identifier: %s", msg); From 0772d187e914929bf2666a1f6acac91639c18765 Mon Sep 17 00:00:00 2001 From: mrmilosz Date: Sun, 13 Dec 2015 01:10:03 -0500 Subject: [PATCH 16/41] Return input tuple in cur.callproc, factor code to use PQescapeIdentifier in single place --- psycopg/cursor_type.c | 43 +++++++++-------------------------------- psycopg/psycopg.h | 1 + psycopg/psycopgmodule.c | 9 +-------- psycopg/utils.c | 33 +++++++++++++++++++++++++++++-- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 0a785c05..e205ba2a 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1015,35 +1015,6 @@ exit: #define psyco_curs_callproc_doc \ "callproc(procname, parameters=None) -- Execute stored procedure." -/* Call PQescapeIdentifier. - * - * In case of error set a Python exception. - * - * TODO: this function can become more generic and go into utils - */ -static char * -_escape_identifier(PGconn *pgconn, const char *str, size_t length) -{ - char *rv = NULL; - -#if PG_VERSION_NUM >= 90000 - rv = PQescapeIdentifier(pgconn, str, length); - if (!rv) { - char *msg; - msg = PQerrorMessage(pgconn); - if (!msg || !msg[0]) { - msg = "no message provided"; - } - PyErr_Format(InterfaceError, "failed to escape identifier: %s", msg); - } -#else - PyErr_Format(PyExc_NotImplementedError, - "named parameters require psycopg2 compiled against libpq 9.0+"); -#endif - - return rv; -} - static PyObject * psyco_curs_callproc(cursorObject *self, PyObject *args) { @@ -1107,7 +1078,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (!(pname = psycopg_ensure_bytes(pname))) { goto exit; } if (!(cpname = Bytes_AsString(pname))) { goto exit; } - if (!(scpnames[i] = _escape_identifier( + if (!(scpnames[i] = psycopg_escape_identifier( self->conn->pgconn, cpname, strlen(cpname)))) { goto exit; } @@ -1158,10 +1129,14 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (0 <= _psyco_curs_execute( self, operation, pvals, self->conn->async, 0)) { - - /* return None from this until it's DBAPI compliant... */ - Py_INCREF(Py_None); - res = Py_None; + /* The dict case is outside DBAPI scope anyway, so simply return None */ + if (using_dict) { + Py_INCREF(Py_None); + res = Py_None; + } + else { + res = pvals; + } } exit: diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index eb406fd2..1a16e8af 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -124,6 +124,7 @@ RAISES HIDDEN PyObject *psyco_set_error(PyObject *exc, cursorObject *curs, const HIDDEN char *psycopg_escape_string(connectionObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); +HIDDEN char *psycopg_escape_identifier(PGconn *pgconn, const char *str, size_t length); HIDDEN int psycopg_strdup(char **to, const char *from, Py_ssize_t len); HIDDEN int psycopg_is_text_file(PyObject *f); diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index cf70a4ad..0ecfcc9c 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -59,7 +59,6 @@ HIDDEN PyObject *pyDateTimeModuleP = NULL; HIDDEN PyObject *psycoEncodings = NULL; - #ifdef PSYCOPG_DEBUG HIDDEN int psycopg_debug_enabled = 0; #endif @@ -175,7 +174,6 @@ exit: static PyObject * psyco_quote_ident(PyObject *self, PyObject *args, PyObject *kwargs) { -#if PG_VERSION_NUM >= 90000 PyObject *ident = NULL, *obj = NULL, *result = NULL; connectionObject *conn; const char *str; @@ -203,9 +201,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->pgconn, str, strlen(str)); if (!quoted) { - PyErr_NoMemory(); goto exit; } result = conn_text_from_chars(conn, quoted); @@ -215,10 +212,6 @@ exit: Py_XDECREF(ident); return result; -#else - PyErr_SetString(NotSupportedError, "PQescapeIdentifier not available in libpq < 9.0"); - return NULL; -#endif } /** type registration **/ diff --git a/psycopg/utils.c b/psycopg/utils.c index ec8e47c8..e224a0d5 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -95,8 +95,8 @@ psycopg_escape_string(connectionObject *conn, const char *from, Py_ssize_t len, * The returned string doesn't include quotes. * * WARNING: this function is not so safe to allow untrusted input: it does no - * check for multibyte chars. Such a function should be built on - * PQescapeIdentifier, which is only available from PostgreSQL 9.0. + * check for multibyte chars. Functions otherwise reliant on PostgreSQL 9.0 + * and above should use the below function psycopg_escape_identifier instead. */ char * psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) @@ -124,6 +124,35 @@ psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) return rv; } + +/* Call PostgreSQL 9.0+ function PQescapeIdentifier. + * + * In case of error set a Python exception. + */ +char * +psycopg_escape_identifier(PGconn *pgconn, const char *str, size_t length) +{ + char *rv = NULL; + +#if PG_VERSION_NUM >= 90000 + rv = PQescapeIdentifier(pgconn, str, length); + if (!rv) { + char *msg; + msg = PQerrorMessage(pgconn); + if (!msg || !msg[0]) { + msg = "no message provided"; + } + PyErr_Format(InterfaceError, "failed to escape identifier: %s", msg); + } +#else + PyErr_Format(PyExc_NotImplementedError, + "PQescapeIdentifier requires psycopg2 compiled against libpq 9.0+"); +#endif + + return rv; +} + + /* Duplicate a string. * * Allocate a new buffer on the Python heap containing the new string. From fb1dbc2a9b308dafa1d8d8e21ef39722d4c6473c Mon Sep 17 00:00:00 2001 From: Christoph Moench-Tegeder Date: Fri, 21 Oct 2016 15:32:11 +0200 Subject: [PATCH 17/41] do not "SET datestyle" on replication connections A replication connection - marked by the use of the keyword "replication" in the DSN - does not support SET commands. Trying to sent "SET datestyle" will result in an exception. --- psycopg/connection_int.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 43d0fdae..c8880b16 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -494,6 +494,26 @@ conn_setup_cancel(connectionObject *self, PGconn *pgconn) return 0; } +/* Return 1 if the "replication" keyword is set in the DSN, 0 otherwise */ +static int +dsn_has_replication(char *pgdsn) +{ + int ret = 0; + PQconninfoOption *connopts, *ptr; + + connopts = PQconninfoParse(pgdsn, NULL); + + for(ptr = connopts; ptr->keyword != NULL; ptr++) { + printf("keyword %s val %s\n", ptr->keyword, ptr->val); + if(strcmp(ptr->keyword, "replication") == 0 && ptr->val != NULL) + ret = 1; + } + + PQconninfoFree(connopts); + + return ret; +} + /* Return 1 if the server datestyle allows us to work without problems, 0 if it needs to be set to something better, e.g. ISO. */ @@ -543,7 +563,7 @@ conn_setup(connectionObject *self, PGconn *pgconn) pthread_mutex_lock(&self->lock); Py_BLOCK_THREADS; - if (!conn_is_datestyle_ok(self->pgconn)) { + if (!dsn_has_replication(self->dsn) && !conn_is_datestyle_ok(self->pgconn)) { int res; Py_UNBLOCK_THREADS; res = pq_set_guc_locked(self, "datestyle", "ISO", @@ -859,8 +879,11 @@ _conn_poll_setup_async(connectionObject *self) self->autocommit = 1; /* If the datestyle is ISO or anything else good, - * we can skip the CONN_STATUS_DATESTYLE step. */ - if (!conn_is_datestyle_ok(self->pgconn)) { + * we can skip the CONN_STATUS_DATESTYLE step. + * Note that we cannot change the datestyle on a replication + * connection. + */ + if (!dsn_has_replication(self->dsn) && !conn_is_datestyle_ok(self->pgconn)) { Dprintf("conn_poll: status -> CONN_STATUS_DATESTYLE"); self->status = CONN_STATUS_DATESTYLE; if (0 == pq_send_query(self, psyco_datestyle)) { From 3971ee6d1fccf831395f396a81afbd65c21b605d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 00:07:23 +0100 Subject: [PATCH 18/41] Testing CI with Travis --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index 1aa25416..09744c20 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,11 @@ language: python +services: + - postgresql + +addons: + postgresql: 9.4 + python: - 2.6 - 2.7 From 0be783c4546c2bbebc67040b212db38ab872d59c Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 00:12:07 +0100 Subject: [PATCH 19/41] Disable email notification Mmm... it seems it's going to be a long night... --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 09744c20..9587a78f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,3 +17,7 @@ install: - python setup.py install script: make check + + +notifications: + email: false From b3cd125d2757872e9337d8df3d8e286345a67450 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 00:18:09 +0100 Subject: [PATCH 20/41] Create the hstore extension in the trevis db --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 9587a78f..027763e2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ python: before_script: - psql -c 'create database psycopg2_test;' -U postgres + - psql -c 'create extension hstore;' -U postgres install: - python setup.py install From a478ba9a4785eac4839f0c4f65e90f6557d42c65 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 00:18:22 +0100 Subject: [PATCH 21/41] Fixed tests failing on Python 2.6 --- tests/test_connection.py | 2 +- tests/test_module.py | 4 ++-- tests/test_quote.py | 10 ++++++---- tests/test_replication.py | 4 ---- tests/testutils.py | 3 +++ 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 8744488d..833751b9 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -465,7 +465,7 @@ class MakeDsnTestCase(ConnectingTestCase): conn = self.connect() d = conn.get_dsn_parameters() self.assertEqual(d['dbname'], dbname) # the only param we can check reliably - self.assertNotIn('password', d) + self.assert_('password' not in d, d) class IsolationLevelsTestCase(ConnectingTestCase): diff --git a/tests/test_module.py b/tests/test_module.py index 1a9a19d4..6a1606d6 100755 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -119,8 +119,8 @@ class ConnectTestCase(unittest.TestCase): def test_int_port_param(self): psycopg2.connect(database='sony', port=6543) dsn = " %s " % self.args[0] - self.assertIn(" dbname=sony ", dsn) - self.assertIn(" port=6543 ", dsn) + self.assert_(" dbname=sony " in dsn, dsn) + self.assert_(" port=6543 " in dsn, dsn) def test_empty_param(self): psycopg2.connect(database='sony', password='') diff --git a/tests/test_quote.py b/tests/test_quote.py index f74fd854..72c9c1e4 100755 --- a/tests/test_quote.py +++ b/tests/test_quote.py @@ -65,11 +65,13 @@ class QuotingTestCase(ConnectingTestCase): curs = self.conn.cursor() data = 'abcd\x01\x00cdefg' - with self.assertRaises(ValueError) as e: + try: curs.execute("SELECT %s", (data,)) - - self.assertEquals(str(e.exception), - 'A string literal cannot contain NUL (0x00) characters.') + except ValueError as e: + self.assertEquals(str(e), + 'A string literal cannot contain NUL (0x00) characters.') + else: + self.fail("ValueError not raised") def test_binary(self): data = b"""some data with \000\013 binary diff --git a/tests/test_replication.py b/tests/test_replication.py index ca99038a..2ccd4c77 100644 --- a/tests/test_replication.py +++ b/tests/test_replication.py @@ -35,11 +35,7 @@ from testutils import ConnectingTestCase class ReplicationTestCase(ConnectingTestCase): def setUp(self): - if not testconfig.repl_dsn: - self.skipTest("replication tests disabled by default") - super(ReplicationTestCase, self).setUp() - self.slot = testconfig.repl_slot self._slots = [] diff --git a/tests/testutils.py b/tests/testutils.py index d0a34bcf..1dd0c999 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -122,6 +122,9 @@ class ConnectingTestCase(unittest.TestCase): Should raise a skip test if not available, but guard for None on old Python versions. """ + if repl_dsn is None: + return self.skipTest("replication tests disabled by default") + if 'dsn' not in kwargs: kwargs['dsn'] = repl_dsn import psycopg2 From 11ad1005e0b03de7eefe883e890a060611bcaede Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 01:09:57 +0100 Subject: [PATCH 22/41] Added python3 supported versions --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index 027763e2..02d96045 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,11 @@ addons: python: - 2.6 - 2.7 + - 3.2 + - 3.3 + - 3.4 + - 3.5 + - 3.6-dev before_script: - psql -c 'create database psycopg2_test;' -U postgres From def22982fb01c6b1411c721ddf0a9f73865d0383 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 02:15:24 +0100 Subject: [PATCH 23/41] Run the tests against all the available server versions --- .travis.yml | 18 +++++++---------- scripts/travis_prepare.sh | 41 +++++++++++++++++++++++++++++++++++++++ scripts/travis_test.sh | 39 +++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 11 deletions(-) create mode 100755 scripts/travis_prepare.sh create mode 100755 scripts/travis_test.sh diff --git a/.travis.yml b/.travis.yml index 02d96045..d41c801b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,9 @@ +# Travis CI configuration file for psycopg2 + +dist: trusty +sudo: required language: python -services: - - postgresql - -addons: - postgresql: 9.4 - python: - 2.6 - 2.7 @@ -15,14 +13,12 @@ python: - 3.5 - 3.6-dev -before_script: - - psql -c 'create database psycopg2_test;' -U postgres - - psql -c 'create extension hstore;' -U postgres - install: - python setup.py install -script: make check +script: + - sudo scripts/travis_prepare.sh + - scripts/travis_test.sh notifications: diff --git a/scripts/travis_prepare.sh b/scripts/travis_prepare.sh new file mode 100755 index 00000000..86b85bae --- /dev/null +++ b/scripts/travis_prepare.sh @@ -0,0 +1,41 @@ +#!/bin/bash + +set -e + +# Prepare the test databases in Travis CI. +# The script should be run with sudo. +# The script is not idempotent: it assumes the machine in a clean state +# and is designed for a sudo-enabled Trusty environment. + +set_param () { + # Set a parameter in a postgresql.conf file + version=$1 + param=$2 + value=$3 + + sed -i "s/^\s*#\?\s*$param.*/$param = $value/" \ + "/etc/postgresql/$version/psycopg/postgresql.conf" +} + +create () { + version=$1 + port=$2 + dbname=psycopg2_test_$port + + pg_createcluster -p $port --start-conf manual $version psycopg + set_param "$version" max_prepared_transactions 10 + sed -i "s/local\s*all\s*postgres.*/local all postgres trust/" \ + "/etc/postgresql/$version/psycopg/pg_hba.conf" + pg_ctlcluster "$version" psycopg start + + sudo -u postgres psql -c "create user travis" "port=$port" +} + +# Would give a permission denied error in the travis build dir +cd / + +create 9.6 54396 +create 9.5 54395 +create 9.4 54394 +create 9.3 54393 +create 9.2 54392 diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh new file mode 100755 index 00000000..3a1bdb28 --- /dev/null +++ b/scripts/travis_test.sh @@ -0,0 +1,39 @@ +#!/bin/bash + +# Run the tests in all the databases +# The script is designed for a Trusty environment. + +set -e + +run_test () { + version=$1 + port=$2 + pyver=$(python -c "import sys; print(''.join(map(str,sys.version_info[:2])))") + dbname=psycopg_test_$pyver + + # Create a database for each python version to allow tests to run in parallel + psql -c "create database $dbname" \ + "user=postgres port=$port dbname=postgres" + + psql -c "grant create on database $dbname to travis" \ + "user=postgres port=$port dbname=postgres" + + psql -c "create extension hstore" \ + "user=postgres port=$port dbname=$dbname" + + printf "\n\nRunning tests against PostgreSQL $version\n\n" + export PSYCOPG2_TESTDB=$dbname + export PSYCOPG2_TESTDB_PORT=$port + export PSYCOPG2_TESTDB_USER=travis + make check + + printf "\n\nRunning tests against PostgreSQL $version (green mode)\n\n" + export PSYCOPG2_TEST_GREEN=1 + make check +} + +run_test 9.6 54396 +run_test 9.5 54395 +run_test 9.4 54394 +run_test 9.3 54393 +run_test 9.2 54392 From 6758ce5eefe7122f1e3e68743504a9ca6b33321c Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 04:27:51 +0100 Subject: [PATCH 24/41] Test Python versions in a more relevant order --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index d41c801b..ef056fa1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,13 +5,13 @@ sudo: required language: python python: - - 2.6 - 2.7 - - 3.2 - - 3.3 - - 3.4 - - 3.5 - 3.6-dev + - 3.5 + - 2.6 + - 3.4 + - 3.3 + - 3.2 install: - python setup.py install From 1463bdb86d46d2d729bf0663443d765ac975f100 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 04:28:34 +0100 Subject: [PATCH 25/41] Added build badge to readme --- README.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.rst b/README.rst index 51d2d6b6..f18be564 100644 --- a/README.rst +++ b/README.rst @@ -44,3 +44,8 @@ For any other resource (source code repository, bug tracker, mailing list) please check the `project homepage`__. .. __: http://initd.org/psycopg/ + + +.. image:: https://travis-ci.org/psycopg/psycopg2.svg?branch=master + :target: https://travis-ci.org/psycopg/psycopg2 + :alt: Build Status From feebc8f68991fc1e84ac4fefb9cea0c373eea6db Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 24 Dec 2016 04:42:07 +0100 Subject: [PATCH 26/41] Don't use separate databases for tests I got this wrong: I thought parallel test ran in the same VM; they are isolated instead. --- .travis.yml | 5 ++--- scripts/travis_prepare.sh | 8 +++++--- scripts/travis_test.sh | 13 +------------ 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index ef056fa1..10411637 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,19 +7,18 @@ language: python python: - 2.7 - 3.6-dev - - 3.5 - 2.6 + - 3.5 - 3.4 - 3.3 - 3.2 install: - python setup.py install + - sudo scripts/travis_prepare.sh script: - - sudo scripts/travis_prepare.sh - scripts/travis_test.sh - notifications: email: false diff --git a/scripts/travis_prepare.sh b/scripts/travis_prepare.sh index 86b85bae..f4e86118 100755 --- a/scripts/travis_prepare.sh +++ b/scripts/travis_prepare.sh @@ -20,17 +20,19 @@ set_param () { create () { version=$1 port=$2 - dbname=psycopg2_test_$port + dbname=psycopg2_test pg_createcluster -p $port --start-conf manual $version psycopg set_param "$version" max_prepared_transactions 10 - sed -i "s/local\s*all\s*postgres.*/local all postgres trust/" \ - "/etc/postgresql/$version/psycopg/pg_hba.conf" pg_ctlcluster "$version" psycopg start sudo -u postgres psql -c "create user travis" "port=$port" + sudo -u postgres psql -c "create database $dbname" "port=$port" + sudo -u postgres psql -c "grant create on database $dbname to travis" "port=$port" + sudo -u postgres psql -c "create extension hstore" "port=$port dbname=$dbname" } + # Would give a permission denied error in the travis build dir cd / diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 3a1bdb28..df9413a1 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -8,18 +8,7 @@ set -e run_test () { version=$1 port=$2 - pyver=$(python -c "import sys; print(''.join(map(str,sys.version_info[:2])))") - dbname=psycopg_test_$pyver - - # Create a database for each python version to allow tests to run in parallel - psql -c "create database $dbname" \ - "user=postgres port=$port dbname=postgres" - - psql -c "grant create on database $dbname to travis" \ - "user=postgres port=$port dbname=postgres" - - psql -c "create extension hstore" \ - "user=postgres port=$port dbname=$dbname" + dbname=psycopg2_test printf "\n\nRunning tests against PostgreSQL $version\n\n" export PSYCOPG2_TESTDB=$dbname From c2d405116b7b68808930eebf7e7b076d8dd17030 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 17:44:25 +0100 Subject: [PATCH 27/41] Dropped testing print --- psycopg/connection_int.c | 1 - 1 file changed, 1 deletion(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index c8880b16..e5c6579f 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -504,7 +504,6 @@ dsn_has_replication(char *pgdsn) connopts = PQconninfoParse(pgdsn, NULL); for(ptr = connopts; ptr->keyword != NULL; ptr++) { - printf("keyword %s val %s\n", ptr->keyword, ptr->val); if(strcmp(ptr->keyword, "replication") == 0 && ptr->val != NULL) ret = 1; } From e27579292aff953dacdc1892f00dc32bb73a29c1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 17:45:01 +0100 Subject: [PATCH 28/41] Avoid deadlock on close if set datestyle failed --- psycopg/connection_int.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index e5c6579f..a34e5ef9 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -541,21 +541,22 @@ conn_setup(connectionObject *self, PGconn *pgconn) { PGresult *pgres = NULL; char *error = NULL; + int rv = -1; self->equote = conn_get_standard_conforming_strings(pgconn); self->server_version = conn_get_server_version(pgconn); self->protocol = conn_get_protocol_version(self->pgconn); if (3 != self->protocol) { PyErr_SetString(InterfaceError, "only protocol 3 supported"); - return -1; + goto exit; } if (0 > conn_read_encoding(self, pgconn)) { - return -1; + goto exit; } if (0 > conn_setup_cancel(self, pgconn)) { - return -1; + goto exit; } Py_BEGIN_ALLOW_THREADS; @@ -570,18 +571,23 @@ conn_setup(connectionObject *self, PGconn *pgconn) Py_BLOCK_THREADS; if (res < 0) { pq_complete_error(self, &pgres, &error); - return -1; + goto unlock; } } /* for reset */ self->autocommit = 0; + /* success */ + rv = 0; + +unlock: Py_UNBLOCK_THREADS; pthread_mutex_unlock(&self->lock); Py_END_ALLOW_THREADS; - return 0; +exit: + return rv; } /* conn_connect - execute a connection to the database */ From b73115ac41559c31fc3a2a3fdb0893046c08d1a5 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 17:46:11 +0100 Subject: [PATCH 29/41] Added test to verify bug #482 --- tests/test_replication.py | 17 +++++++++++++++-- tests/testutils.py | 11 ++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) mode change 100644 => 100755 tests/test_replication.py diff --git a/tests/test_replication.py b/tests/test_replication.py old mode 100644 new mode 100755 index 2ccd4c77..33a8065a --- a/tests/test_replication.py +++ b/tests/test_replication.py @@ -23,7 +23,6 @@ # License for more details. import psycopg2 -import psycopg2.extensions from psycopg2.extras import ( PhysicalReplicationConnection, LogicalReplicationConnection, StopReplication) @@ -89,6 +88,20 @@ class ReplicationTest(ReplicationTestCase): cur.execute("IDENTIFY_SYSTEM") cur.fetchall() + @skip_before_postgres(9, 0) + def test_datestyle(self): + if testconfig.repl_dsn is None: + return self.skipTest("replication tests disabled by default") + + conn = self.repl_connect( + dsn=testconfig.repl_dsn, options='-cdatestyle=german', + connection_factory=PhysicalReplicationConnection) + if conn is None: + return + cur = conn.cursor() + cur.execute("IDENTIFY_SYSTEM") + cur.fetchall() + @skip_before_postgres(9, 4) def test_logical_replication_connection(self): conn = self.repl_connect(connection_factory=LogicalReplicationConnection) @@ -168,7 +181,7 @@ class AsyncReplicationTest(ReplicationTestCase): connection_factory=LogicalReplicationConnection, async=1) if conn is None: return - self.wait(conn) + cur = conn.cursor() self.create_replication_slot(cur, output_plugin='test_decoding') diff --git a/tests/testutils.py b/tests/testutils.py index 1dd0c999..93477357 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -130,8 +130,17 @@ class ConnectingTestCase(unittest.TestCase): import psycopg2 try: conn = self.connect(**kwargs) + if conn.async == 1: + self.wait(conn) except psycopg2.OperationalError, e: - return self.skipTest("replication db not configured: %s" % e) + # If pgcode is not set it is a genuine connection error + # Otherwise we tried to run some bad operation in the connection + # (e.g. bug #482) and we'd rather know that. + if e.pgcode is None: + return self.skipTest("replication db not configured: %s" % e) + else: + raise + return conn def _get_conn(self): From 874705db429de5cc23a20c5e5cb85287c163f037 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 17:49:58 +0100 Subject: [PATCH 30/41] Configure Travis to test replication --- scripts/travis_prepare.sh | 19 ++++++++++++++++++- scripts/travis_test.sh | 6 ++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/scripts/travis_prepare.sh b/scripts/travis_prepare.sh index f4e86118..2b1e12eb 100755 --- a/scripts/travis_prepare.sh +++ b/scripts/travis_prepare.sh @@ -23,10 +23,27 @@ create () { dbname=psycopg2_test pg_createcluster -p $port --start-conf manual $version psycopg + + # for two-phase commit testing set_param "$version" max_prepared_transactions 10 + + # for replication testing + set_param "$version" max_wal_senders 5 + set_param "$version" max_replication_slots 5 + if [ "$version" == "9.2" -o "$version" == "9.3" ] + then + set_param "$version" wal_level hot_standby + else + set_param "$version" wal_level logical + fi + + echo "local replication travis trust" \ + >> "/etc/postgresql/$version/psycopg/pg_hba.conf" + + pg_ctlcluster "$version" psycopg start - sudo -u postgres psql -c "create user travis" "port=$port" + sudo -u postgres psql -c "create user travis replication" "port=$port" sudo -u postgres psql -c "create database $dbname" "port=$port" sudo -u postgres psql -c "grant create on database $dbname to travis" "port=$port" sudo -u postgres psql -c "create extension hstore" "port=$port dbname=$dbname" diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index df9413a1..15783088 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -14,11 +14,13 @@ run_test () { export PSYCOPG2_TESTDB=$dbname export PSYCOPG2_TESTDB_PORT=$port export PSYCOPG2_TESTDB_USER=travis - make check + export PSYCOPG2_TEST_REPL_DSN= + + python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" --verbose printf "\n\nRunning tests against PostgreSQL $version (green mode)\n\n" export PSYCOPG2_TEST_GREEN=1 - make check + python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" --verbose } run_test 9.6 54396 From c22093ddd49ea6045e05b9eaafafc7a001bac1a5 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 19:00:30 +0100 Subject: [PATCH 31/41] Skip replication tests in green mode --- tests/test_replication.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_replication.py b/tests/test_replication.py index 33a8065a..79d1295d 100755 --- a/tests/test_replication.py +++ b/tests/test_replication.py @@ -27,9 +27,10 @@ from psycopg2.extras import ( PhysicalReplicationConnection, LogicalReplicationConnection, StopReplication) import testconfig -from testutils import unittest -from testutils import skip_before_postgres -from testutils import ConnectingTestCase +from testutils import unittest, ConnectingTestCase +from testutils import skip_before_postgres, skip_if_green + +skip_repl_if_green = skip_if_green("replication not supported in green mode") class ReplicationTestCase(ConnectingTestCase): @@ -123,6 +124,7 @@ class ReplicationTest(ReplicationTestCase): psycopg2.ProgrammingError, self.create_replication_slot, cur) @skip_before_postgres(9, 4) # slots require 9.4 + @skip_repl_if_green def test_start_on_missing_replication_slot(self): conn = self.repl_connect(connection_factory=PhysicalReplicationConnection) if conn is None: @@ -136,6 +138,7 @@ class ReplicationTest(ReplicationTestCase): cur.start_replication(self.slot) @skip_before_postgres(9, 4) # slots require 9.4 + @skip_repl_if_green def test_start_and_recover_from_error(self): conn = self.repl_connect(connection_factory=LogicalReplicationConnection) if conn is None: @@ -157,6 +160,7 @@ class ReplicationTest(ReplicationTestCase): cur.start_replication(slot_name=self.slot) @skip_before_postgres(9, 4) # slots require 9.4 + @skip_repl_if_green def test_stop_replication(self): conn = self.repl_connect(connection_factory=LogicalReplicationConnection) if conn is None: @@ -176,6 +180,7 @@ class ReplicationTest(ReplicationTestCase): class AsyncReplicationTest(ReplicationTestCase): @skip_before_postgres(9, 4) # slots require 9.4 + @skip_repl_if_green def test_async_replication(self): conn = self.repl_connect( connection_factory=LogicalReplicationConnection, async=1) From d48d4bab0520d047da1522e12ddaa51702e231d0 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 7 Jul 2016 12:05:29 +0100 Subject: [PATCH 32/41] Added empty options in setup.cfg Setuptools removes them from the sdist, see #453 --- setup.cfg | 15 +++++++-------- setup.py | 7 ++++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/setup.cfg b/setup.cfg index 90a47dd4..0d41934f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,24 +7,23 @@ define= # "pg_config" is required to locate PostgreSQL headers and libraries needed to # build psycopg2. If pg_config is not in the path or is installed under a -# different name uncomment the following option and set it to the pg_config -# full path. -#pg_config= +# different name set the following option to the pg_config full path. +pg_config= # Set to 1 to use Python datetime objects for default date/time representation. use_pydatetime=1 # If the build system does not find the mx.DateTime headers, try -# uncommenting the following line and setting its value to the right path. -#mx_include_dir= +# setting its value to the right path. +mx_include_dir= # For Windows only: # Set to 1 if the PostgreSQL library was built with OpenSSL. # Required to link in OpenSSL libraries and dependencies. have_ssl=0 -# Statically link against the postgresql client library. -#static_libpq=1 +# Set to 1 to statically link against the postgresql client library. +static_libpq=0 # Add here eventual extra libraries required to link the module. -#libraries= +libraries= diff --git a/setup.py b/setup.py index 3f021830..c1065258 100644 --- a/setup.py +++ b/setup.py @@ -381,6 +381,11 @@ class psycopg_build_ext(build_ext): def finalize_options(self): """Complete the build system configuration.""" + # An empty option in the setup.cfg causes self.libraries to include + # an empty string in the list of libraries + if self.libraries is not None and not self.libraries.strip(): + self.libraries = None + build_ext.finalize_options(self) pg_config_helper = PostgresConfig(self) @@ -521,7 +526,7 @@ if parser.has_option('build_ext', 'mx_include_dir'): mxincludedir = parser.get('build_ext', 'mx_include_dir') else: mxincludedir = os.path.join(get_python_inc(plat_specific=1), "mx") -if os.path.exists(mxincludedir): +if mxincludedir.strip() and os.path.exists(mxincludedir): # Build the support for mx: we will check at runtime if it can be imported include_dirs.append(mxincludedir) define_macros.append(('HAVE_MXDATETIME', '1')) From 35b4a01b6d941d7d6f1b8040db6037c53f713aa1 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Sat, 17 Sep 2016 15:36:19 -0400 Subject: [PATCH 33/41] Fix "invalid escape sequence" warning in Python 3.6 http://bugs.python.org/issue27364 --- lib/extras.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/extras.py b/lib/extras.py index 5c4f5d2a..7fc853a6 100644 --- a/lib/extras.py +++ b/lib/extras.py @@ -908,7 +908,7 @@ WHERE typname = 'hstore'; def register_hstore(conn_or_curs, globally=False, unicode=False, oid=None, array_oid=None): - """Register adapter and typecaster for `!dict`\-\ |hstore| conversions. + r"""Register adapter and typecaster for `!dict`\-\ |hstore| conversions. :param conn_or_curs: a connection or cursor: the typecaster will be registered only on this object unless *globally* is set to `!True` From dcb198e8b7b15b7f3c6f39d9879cb0a2474bc41d Mon Sep 17 00:00:00 2001 From: Luke Nezda Date: Mon, 12 Sep 2016 09:19:20 -0500 Subject: [PATCH 34/41] fix wait_select sample to be `extras` not `extensions` --- doc/src/faq.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/faq.rst b/doc/src/faq.rst index d0636669..89d8a639 100644 --- a/doc/src/faq.rst +++ b/doc/src/faq.rst @@ -241,7 +241,7 @@ How do I interrupt a long-running query in an interactive shell? .. code-block:: pycon - >>> psycopg2.extensions.set_wait_callback(psycopg2.extensions.wait_select) + >>> psycopg2.extensions.set_wait_callback(psycopg2.extras.wait_select) >>> cnn = psycopg2.connect('') >>> cur = cnn.cursor() >>> cur.execute("select pg_sleep(10)") From 4c99cadabe176ef46573378e7cb8bd71b63bd0f3 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 20:55:01 +0100 Subject: [PATCH 35/41] Fixed intersphinx links to Pyton docs --- doc/src/conf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/conf.py b/doc/src/conf.py index 22c5c46f..a918c08c 100644 --- a/doc/src/conf.py +++ b/doc/src/conf.py @@ -61,8 +61,8 @@ except ImportError: release = version intersphinx_mapping = { - 'py': ('http://docs.python.org/', None), - 'py3': ('http://docs.python.org/3.4', None), + 'py': ('http://docs.python.org/2', None), + 'py3': ('http://docs.python.org/3', None), } # Pattern to generate links to the bug tracker From 17698c481566c0e8c1d5d65fe88e0cb7e4505957 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 25 Dec 2016 20:55:15 +0100 Subject: [PATCH 36/41] Fixed REst error in newsfile --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index b7efef29..67883d74 100644 --- a/NEWS +++ b/NEWS @@ -36,7 +36,7 @@ What's new in psycopg 2.6.3 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - Throw an exception trying to pass ``NULL`` chars as parameters - (:ticket:`#420). + (:ticket:`#420`). - Make `~psycopg2.extras.Range` objects picklable (:ticket:`#462`). From a53b39efe8f8bef6f574de24a28fb27b143e890a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 26 Dec 2016 02:37:36 +0100 Subject: [PATCH 37/41] Dropped internal escape identifier function Using libpq one as now it's guaranteed to be present. --- psycopg/cursor.h | 1 + psycopg/cursor_type.c | 48 +++++++++++++++++++++++-------------------- psycopg/psycopg.h | 3 ++- psycopg/utils.c | 40 ++++++++++++++++-------------------- 4 files changed, 47 insertions(+), 45 deletions(-) diff --git a/psycopg/cursor.h b/psycopg/cursor.h index e291d45f..3c94fe3d 100644 --- a/psycopg/cursor.h +++ b/psycopg/cursor.h @@ -80,6 +80,7 @@ struct cursorObject { char *qattr; /* quoting attr, used when quoting strings */ char *notice; /* a notice from the backend */ char *name; /* this cursor name */ + char *qname; /* this cursor name, quoted */ PyObject *string_types; /* a set of typecasters for string types */ PyObject *binary_types; /* a set of typecasters for binary types */ diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index fe79bbf9..6b66217c 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -55,7 +55,7 @@ psyco_curs_close(cursorObject *self) goto exit; } - if (self->name != NULL) { + if (self->qname != NULL) { char buffer[128]; PGTransactionStatusType status; @@ -68,7 +68,7 @@ psyco_curs_close(cursorObject *self) if (!(status == PQTRANS_UNKNOWN || status == PQTRANS_INERROR)) { EXC_IF_NO_MARK(self); - PyOS_snprintf(buffer, 127, "CLOSE \"%s\"", self->name); + PyOS_snprintf(buffer, 127, "CLOSE %s", self->qname); if (pq_execute(self, buffer, 0, 0, 1) == -1) return NULL; } else { @@ -422,10 +422,10 @@ _psyco_curs_execute(cursorObject *self, goto exit; } - if (self->name != NULL) { + if (self->qname != NULL) { self->query = Bytes_FromFormat( - "DECLARE \"%s\" %sCURSOR %s HOLD FOR %s", - self->name, + "DECLARE %s %sCURSOR %s HOLD FOR %s", + self->qname, scroll, self->withhold ? "WITH" : "WITHOUT", Bytes_AS_STRING(fquery)); @@ -436,10 +436,10 @@ _psyco_curs_execute(cursorObject *self, } } else { - if (self->name != NULL) { + if (self->qname != NULL) { self->query = Bytes_FromFormat( - "DECLARE \"%s\" %sCURSOR %s HOLD FOR %s", - self->name, + "DECLARE %s %sCURSOR %s HOLD FOR %s", + self->qname, scroll, self->withhold ? "WITH" : "WITHOUT", Bytes_AS_STRING(operation)); @@ -768,13 +768,13 @@ psyco_curs_fetchone(cursorObject *self) if (_psyco_curs_prefetch(self) < 0) return NULL; EXC_IF_NO_TUPLES(self); - if (self->name != NULL) { + if (self->qname != NULL) { char buffer[128]; 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->name); + PyOS_snprintf(buffer, 127, "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; } @@ -823,8 +823,8 @@ psyco_curs_next_named(cursorObject *self) if (self->row >= self->rowcount) { char buffer[128]; - PyOS_snprintf(buffer, 127, "FETCH FORWARD %ld FROM \"%s\"", - self->itersize, self->name); + PyOS_snprintf(buffer, 127, "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; } @@ -886,14 +886,14 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) if (_psyco_curs_prefetch(self) < 0) return NULL; EXC_IF_NO_TUPLES(self); - if (self->name != NULL) { + if (self->qname != NULL) { char buffer[128]; 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\"", - (int)size, self->name); + PyOS_snprintf(buffer, 127, "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; } } @@ -962,13 +962,13 @@ psyco_curs_fetchall(cursorObject *self) if (_psyco_curs_prefetch(self) < 0) return NULL; EXC_IF_NO_TUPLES(self); - if (self->name != NULL) { + if (self->qname != NULL) { char buffer[128]; 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->name); + PyOS_snprintf(buffer, 127, "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; } } @@ -1153,7 +1153,7 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) /* if the cursor is not named we have the full result set and we can do our own calculations to scroll; else we just delegate the scrolling to the MOVE SQL statement */ - if (self->name == NULL) { + if (self->qname == NULL) { if (strcmp(mode, "relative") == 0) { newpos = self->row + value; } else if (strcmp( mode, "absolute") == 0) { @@ -1181,11 +1181,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\"", - value, self->name); + PyOS_snprintf(buffer, 127, "MOVE ABSOLUTE %d FROM %s", + value, self->qname); } else { - PyOS_snprintf(buffer, 127, "MOVE %d FROM \"%s\"", value, self->name); + PyOS_snprintf(buffer, 127, "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; @@ -1815,7 +1815,10 @@ cursor_setup(cursorObject *self, connectionObject *conn, const char *name) Dprintf("cursor_setup: parameters: name = %s, conn = %p", name, conn); if (name) { - if (!(self->name = psycopg_escape_identifier_easy(name, 0))) { + if (0 > psycopg_strdup(&self->name, name, 0)) { + return -1; + } + if (!(self->qname = psycopg_escape_identifier(conn, name, 0))) { return -1; } } @@ -1891,6 +1894,7 @@ cursor_dealloc(PyObject* obj) cursor_clear(self); PyMem_Free(self->name); + PQfreemem(self->qname); CLEARPGRES(self->pgres); diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 82b4293c..438d7636 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -128,7 +128,8 @@ RAISES HIDDEN PyObject *psyco_set_error(PyObject *exc, cursorObject *curs, const HIDDEN char *psycopg_escape_string(connectionObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); -HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); +HIDDEN char *psycopg_escape_identifier(connectionObject *conn, + const char *str, size_t len); HIDDEN int psycopg_strdup(char **to, const char *from, Py_ssize_t len); HIDDEN int psycopg_is_text_file(PyObject *f); diff --git a/psycopg/utils.c b/psycopg/utils.c index 631b8394..c518cc57 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -90,40 +90,36 @@ psycopg_escape_string(connectionObject *conn, const char *from, Py_ssize_t len, return to; } -/* Escape a string to build a valid PostgreSQL identifier. +/* Escape a string for inclusion in a query as identifier. * - * Allocate a new buffer on the Python heap containing the new string. * 'len' is optional: if 0 the length is calculated. * - * The returned string doesn't include quotes. - * - * WARNING: this function is not so safe to allow untrusted input: it does no - * check for multibyte chars. Such a function should be built on - * PQescapeIdentifier, which is only available from PostgreSQL 9.0. + * Return a string allocated by Postgres: free it using PQfreemem + * In case of error set a Python exception. */ char * -psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) +psycopg_escape_identifier(connectionObject *conn, const char *str, size_t len) { - char *rv; - const char *src; - char *dst; + char *rv = NULL; - if (!len) { len = strlen(from); } - if (!(rv = PyMem_New(char, 1 + 2 * len))) { - PyErr_NoMemory(); - return NULL; + if (!conn || !conn->pgconn) { + PyErr_SetString(InterfaceError, "connection not valid"); + goto exit; } - /* The only thing to do is double quotes */ - for (src = from, dst = rv; *src; ++src, ++dst) { - *dst = *src; - if ('"' == *src) { - *++dst = '"'; + if (!len) { len = strlen(str); } + + rv = PQescapeIdentifier(conn->pgconn, str, len); + if (!rv) { + char *msg; + msg = PQerrorMessage(conn->pgconn); + if (!msg || !msg[0]) { + msg = "no message provided"; } + PyErr_Format(InterfaceError, "failed to escape identifier: %s", msg); } - *dst = '\0'; - +exit: return rv; } From d13521a6ce1f92956dcca2b68e4e703008580c7b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 26 Dec 2016 03:39:28 +0100 Subject: [PATCH 38/41] Mention named callproc in news, fixed docs. --- NEWS | 1 + doc/src/cursor.rst | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) 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 974e1a2b..aee6b465 100644 --- a/doc/src/cursor.rst +++ b/doc/src/cursor.rst @@ -202,8 +202,7 @@ 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. Overloaded procedures are supported. Named parameters can be - used with a PostgreSQL 9.0+ client by supplying the sequence of - parameters as a Dict. + 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 @@ -213,6 +212,8 @@ The ``cursor`` class 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]) From ffeb7001ebfaab34613ce604a509dfa1de193b80 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 26 Dec 2016 04:12:18 +0100 Subject: [PATCH 39/41] Fixed refcount problems in named callproc --- psycopg/cursor_type.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 66580ad5..baa5b8f7 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1080,6 +1080,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) if (!(scpnames[i] = psycopg_escape_identifier( self->conn, cpname, 0))) { + Py_CLEAR(pname); goto exit; } @@ -1131,12 +1132,12 @@ psyco_curs_callproc(cursorObject *self, PyObject *args) self, operation, pvals, self->conn->async, 0)) { /* The dict case is outside DBAPI scope anyway, so simply return None */ if (using_dict) { - Py_INCREF(Py_None); res = Py_None; } else { res = pvals; } + Py_INCREF(res); } exit: From 64342fcff069327034aa7d21b6faa91dccd0d69e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 26 Dec 2016 03:47:13 +0100 Subject: [PATCH 40/41] Less verbose travis tests --- scripts/travis_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 15783088..61b57f6c 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -16,11 +16,11 @@ run_test () { export PSYCOPG2_TESTDB_USER=travis export PSYCOPG2_TEST_REPL_DSN= - python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" --verbose + python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" printf "\n\nRunning tests against PostgreSQL $version (green mode)\n\n" export PSYCOPG2_TEST_GREEN=1 - python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" --verbose + python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" } run_test 9.6 54396 From c46b6ea719b8a1cd97ed0161de6b6e31e0319d2b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 26 Dec 2016 04:31:18 +0100 Subject: [PATCH 41/41] Fixed travis test: unset green mode --- scripts/travis_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 61b57f6c..0c60b937 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -15,7 +15,7 @@ run_test () { export PSYCOPG2_TESTDB_PORT=$port export PSYCOPG2_TESTDB_USER=travis export PSYCOPG2_TEST_REPL_DSN= - + unset PSYCOPG2_TEST_GREEN python -c "from psycopg2 import tests; tests.unittest.main(defaultTest='tests.test_suite')" printf "\n\nRunning tests against PostgreSQL $version (green mode)\n\n"