From 6c5ad8d63c5513c4e81fa2d95bc0c6b359150765 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 17:25:28 +0900 Subject: [PATCH 1/8] Optimize dict fetch --- MySQLdb/_mysql.c | 107 +++++++++++++++++++++++++++++++++---------- tests/test_cursor.py | 28 +++++++++++ 2 files changed, 112 insertions(+), 23 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index a58cf8f7..075a3b9d 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1194,7 +1194,8 @@ _mysql_field_to_python( static PyObject * _mysql_row_to_tuple( _mysql_ResultObject *self, - MYSQL_ROW row) + MYSQL_ROW row, + PyObject *unused) { unsigned int n, i; unsigned long *length; @@ -1221,7 +1222,8 @@ _mysql_row_to_tuple( static PyObject * _mysql_row_to_dict( _mysql_ResultObject *self, - MYSQL_ROW row) + MYSQL_ROW row, + PyObject *cache) { unsigned int n, i; unsigned long *length; @@ -1245,15 +1247,20 @@ _mysql_row_to_dict( } PyObject *tmp = PyDict_SetDefault(r, pyname, v); + if (tmp == v) { + if (cache) { + PyTuple_SET_ITEM(cache, i, pyname); + } else { + Py_DECREF(pyname); + } + Py_DECREF(v); + continue; + } Py_DECREF(pyname); if (!tmp) { Py_DECREF(v); goto error; } - if (tmp == v) { - Py_DECREF(v); - continue; - } pyname = PyUnicode_FromFormat("%s.%s", fields[i].table, fields[i].name); if (!pyname) { @@ -1261,7 +1268,11 @@ _mysql_row_to_dict( goto error; } int err = PyDict_SetItem(r, pyname, v); - Py_DECREF(pyname); + if (cache) { + PyTuple_SET_ITEM(cache, i, pyname); + } else { + Py_DECREF(pyname); + } Py_DECREF(v); if (err) { goto error; @@ -1276,7 +1287,8 @@ _mysql_row_to_dict( static PyObject * _mysql_row_to_dict_old( _mysql_ResultObject *self, - MYSQL_ROW row) + MYSQL_ROW row, + PyObject *cache) { unsigned int n, i; unsigned long *length; @@ -1302,8 +1314,12 @@ _mysql_row_to_dict_old( pyname = PyUnicode_FromString(fields[i].name); } int err = PyDict_SetItem(r, pyname, v); - Py_DECREF(pyname); Py_DECREF(v); + if (cache) { + PyTuple_SET_ITEM(cache, i, pyname); + } else { + Py_DECREF(pyname); + } if (err) { goto error; } @@ -1314,15 +1330,66 @@ _mysql_row_to_dict_old( return NULL; } -typedef PyObject *_PYFUNC(_mysql_ResultObject *, MYSQL_ROW); +static PyObject * +_mysql_row_to_dict_cached( + _mysql_ResultObject *self, + MYSQL_ROW row, + PyObject *cache) +{ + PyObject *r = PyDict_New(); + if (!r) { + return NULL; + } + + unsigned int n = mysql_num_fields(self->result); + unsigned long *length = mysql_fetch_lengths(self->result); + MYSQL_FIELD *fields = mysql_fetch_fields(self->result); + + for (unsigned int i=0; iconverter, i); + PyObject *v = _mysql_field_to_python(c, row[i], length[i], &fields[i], self->encoding); + if (!v) { + goto error; + } + + PyObject *pyname = PyTuple_GET_ITEM(cache, i); // borrowed + int err = PyDict_SetItem(r, pyname, v); + Py_DECREF(v); + if (err) { + goto error; + } + } + return r; + error: + Py_XDECREF(r); + return NULL; +} + + +typedef PyObject *_convertfunc(_mysql_ResultObject *, MYSQL_ROW, PyObject *); +static _convertfunc * const row_converters[] = { + _mysql_row_to_tuple, + _mysql_row_to_dict, + _mysql_row_to_dict_old +}; Py_ssize_t _mysql__fetch_row( _mysql_ResultObject *self, PyObject *r, /* list object */ Py_ssize_t maxrows, - _PYFUNC *convert_row) + int how) { + _convertfunc *convert_row = row_converters[how]; + + PyObject *cache = NULL; + if (maxrows > 0 && how > 1) { + cache = PyTuple_New(mysql_num_fields(self->result)); + if (!cache) { + return -1; + } + } + Py_ssize_t i; for (i = 0; i < maxrows; i++) { MYSQL_ROW row; @@ -1340,8 +1407,11 @@ _mysql__fetch_row( if (!row) { break; } - PyObject *v = convert_row(self, row); + PyObject *v = convert_row(self, row, cache); if (!v) return -1; + if (cache) { + convert_row = _mysql_row_to_dict_cached; + } if (PyList_Append(r, v)) { Py_DECREF(v); return -1; @@ -1366,15 +1436,7 @@ _mysql_ResultObject_fetch_row( PyObject *args, PyObject *kwargs) { - typedef PyObject *_PYFUNC(_mysql_ResultObject *, MYSQL_ROW); - static char *kwlist[] = { "maxrows", "how", NULL }; - static _PYFUNC *row_converters[] = - { - _mysql_row_to_tuple, - _mysql_row_to_dict, - _mysql_row_to_dict_old - }; - _PYFUNC *convert_row; + static char *kwlist[] = {"maxrows", "how", NULL }; int maxrows=1, how=0; PyObject *r=NULL; @@ -1386,7 +1448,6 @@ _mysql_ResultObject_fetch_row( PyErr_SetString(PyExc_ValueError, "how out of range"); return NULL; } - convert_row = row_converters[how]; if (!maxrows) { if (self->use) { maxrows = INT_MAX; @@ -1396,7 +1457,7 @@ _mysql_ResultObject_fetch_row( } } if (!(r = PyList_New(0))) goto error; - Py_ssize_t rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); + Py_ssize_t rowsadded = _mysql__fetch_row(self, r, maxrows, how); if (rowsadded == -1) goto error; /* DB-API allows return rows as list. diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 479f3e27..9ddcd4f3 100644 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -111,3 +111,31 @@ def test_pyparam(): assert cursor._executed == b"SELECT 1, 2" cursor.execute(b"SELECT %(a)s, %(b)s", {b"a": 3, b"b": 4}) assert cursor._executed == b"SELECT 3, 4" + + +def test_dictcursor(): + conn = connect() + cursor = conn.cursor(MySQLdb.cursors.DictCursor) + + cursor.execute("CREATE TABLE t1 (a int, b int)") + _tables.append("t1") + cursor.execute("INSERT INTO t1 (a,b) VALUES (1,1), (2,2)") + + cursor.execute("CREATE TABLE t2 (b int, c int)") + _tables.append("t2") + cursor.execute("INSERT INTO t2 (b,c) VALUES (1,1), (2,2)") + + cursor.execute("SELECT * FROM t1, t2 ON t1.b=t2.b") + rows = cursor.fetchall() + + assert len(rows) == 2 + assert rows[0] == {"a": 1, "t1.b": 1, "t2.b": 1, "c": 1} + assert rows[1] == {"a": 2, "t1.b": 2, "t2.b": 2, "c": 2} + + # Old fetchtype + cursor.execute("SELECT * FROM t1, t2 ON t1.b=t2.b") + rows = cursor._result.fetch_row(0, 2) + + assert len(rows) == 2 + assert rows[0] == {"t1.a": 1, "t1.b": 1, "t2.b": 1, "t2.c": 1} + assert rows[1] == {"t1.a": 2, "t1.b": 2, "t2.b": 2, "t2.c": 2} From 2ab2d3f0767aa4e84a554b31f4bff91ad162a493 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 17:31:20 +0900 Subject: [PATCH 2/8] fix leak --- MySQLdb/_mysql.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index 075a3b9d..b98377ae 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1402,23 +1402,29 @@ _mysql__fetch_row( } if (!row && mysql_errno(&(((_mysql_ConnectionObject *)(self->conn))->connection))) { _mysql_Exception((_mysql_ConnectionObject *)self->conn); - return -1; + goto error; } if (!row) { break; } PyObject *v = convert_row(self, row, cache); - if (!v) return -1; + if (!v) { + goto error; + } if (cache) { convert_row = _mysql_row_to_dict_cached; } if (PyList_Append(r, v)) { Py_DECREF(v); - return -1; + goto error; } Py_DECREF(v); } + Py_XDECREF(cache); return i; +error: + Py_XDECREF(cache); + return -1; } static char _mysql_ResultObject_fetch_row__doc__[] = From 01732b95a7b51afc00b2ce25f28942e6f4741b54 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 17:34:33 +0900 Subject: [PATCH 3/8] fix sql --- .github/workflows/windows.yaml | 2 ++ tests/test_cursor.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/windows.yaml b/.github/workflows/windows.yaml index a560d48f..c65ce188 100644 --- a/.github/workflows/windows.yaml +++ b/.github/workflows/windows.yaml @@ -2,6 +2,8 @@ name: Build windows wheels on: push: + branches: + - master workflow_dispatch: jobs: diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 9ddcd4f3..6413041f 100644 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -125,7 +125,7 @@ def test_dictcursor(): _tables.append("t2") cursor.execute("INSERT INTO t2 (b,c) VALUES (1,1), (2,2)") - cursor.execute("SELECT * FROM t1, t2 ON t1.b=t2.b") + cursor.execute("SELECT * FROM t1 JOIN t2 ON t1.b=t2.b") rows = cursor.fetchall() assert len(rows) == 2 @@ -133,7 +133,7 @@ def test_dictcursor(): assert rows[1] == {"a": 2, "t1.b": 2, "t2.b": 2, "c": 2} # Old fetchtype - cursor.execute("SELECT * FROM t1, t2 ON t1.b=t2.b") + cursor.execute("SELECT * FROM t1 JOIN t2 ON t1.b=t2.b") rows = cursor._result.fetch_row(0, 2) assert len(rows) == 2 From 9e55652fe3ff9070d5227d93b0686b4c3f4dd2d1 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 17:52:34 +0900 Subject: [PATCH 4/8] fix bug --- MySQLdb/_mysql.c | 34 +++++++++++++--------------------- tests/test_cursor.py | 8 ++++---- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index b98377ae..ae848780 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1245,29 +1245,21 @@ _mysql_row_to_dict( Py_DECREF(v); goto error; } - - PyObject *tmp = PyDict_SetDefault(r, pyname, v); - if (tmp == v) { - if (cache) { - PyTuple_SET_ITEM(cache, i, pyname); - } else { - Py_DECREF(pyname); - } - Py_DECREF(v); - continue; - } - Py_DECREF(pyname); - if (!tmp) { + int err = PyDict_Contains(r, pyname); + if (res < 0) { Py_DECREF(v); goto error; } - - pyname = PyUnicode_FromFormat("%s.%s", fields[i].table, fields[i].name); - if (!pyname) { - Py_DECREF(v); - goto error; + if (res) { + Py_DECREF(pyname); + pyname = PyUnicode_FromFormat("%s.%s", fields[i].table, fields[i].name); + if (pyname == NULL) { + Py_DECREF(v); + goto error; + } } - int err = PyDict_SetItem(r, pyname, v); + + err = PyDict_SetItem(r, pyname, v); if (cache) { PyTuple_SET_ITEM(cache, i, pyname); } else { @@ -1279,8 +1271,8 @@ _mysql_row_to_dict( } } return r; - error: - Py_XDECREF(r); +error: + Py_DECREF(r); return NULL; } diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 6413041f..19c6f61e 100644 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -117,9 +117,9 @@ def test_dictcursor(): conn = connect() cursor = conn.cursor(MySQLdb.cursors.DictCursor) - cursor.execute("CREATE TABLE t1 (a int, b int)") + cursor.execute("CREATE TABLE t1 (a int, b int, c int)") _tables.append("t1") - cursor.execute("INSERT INTO t1 (a,b) VALUES (1,1), (2,2)") + cursor.execute("INSERT INTO t1 (a,b,c) VALUES (1,1,47), (2,2,47)") cursor.execute("CREATE TABLE t2 (b int, c int)") _tables.append("t2") @@ -129,8 +129,8 @@ def test_dictcursor(): rows = cursor.fetchall() assert len(rows) == 2 - assert rows[0] == {"a": 1, "t1.b": 1, "t2.b": 1, "c": 1} - assert rows[1] == {"a": 2, "t1.b": 2, "t2.b": 2, "c": 2} + assert rows[0] == {"a": 1, "b": 1, "c": 47, "t2.c": 1} + assert rows[1] == {"a": 2, "b": 2, "c": 47, "t2.c": 2} # Old fetchtype cursor.execute("SELECT * FROM t1 JOIN t2 ON t1.b=t2.b") From 6a8da543904ec79247620c1c6b78e9eebba9d5f9 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 17:56:58 +0900 Subject: [PATCH 5/8] fix test --- tests/test_cursor.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 19c6f61e..5e88a9a2 100644 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -129,13 +129,23 @@ def test_dictcursor(): rows = cursor.fetchall() assert len(rows) == 2 - assert rows[0] == {"a": 1, "b": 1, "c": 47, "t2.c": 1} - assert rows[1] == {"a": 2, "b": 2, "c": 47, "t2.c": 2} + assert rows[0] == {"a": 1, "b": 1, "c": 47, "t2.b": 1, "t2.c": 1} + assert rows[1] == {"a": 2, "b": 2, "c": 47, "t2.b": 2, "t2.c": 2} + + names1 = sorted(rows[0]) + names2 = sorted(rows[1]) + for a, b in zip(names1, names2): + assert a is b # Old fetchtype cursor.execute("SELECT * FROM t1 JOIN t2 ON t1.b=t2.b") rows = cursor._result.fetch_row(0, 2) assert len(rows) == 2 - assert rows[0] == {"t1.a": 1, "t1.b": 1, "t2.b": 1, "t2.c": 1} - assert rows[1] == {"t1.a": 2, "t1.b": 2, "t2.b": 2, "t2.c": 2} + assert rows[0] == {"t1.a": 1, "t1.b": 1, "t1.c": 47, "t2.b": 1, "t2.c": 1} + assert rows[1] == {"t1.a": 2, "t1.b": 2, "t1.c": 47, "t2.b": 2, "t2.c": 2} + + names1 = sorted(rows[0]) + names2 = sorted(rows[1]) + for a, b in zip(names1, names2): + assert a is b From 9f5ecdc36db85f16b00a33a0b2aa3ebedeb9cbef Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 18:00:27 +0900 Subject: [PATCH 6/8] fix C code --- MySQLdb/_mysql.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index ae848780..f8f81a58 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1246,11 +1246,11 @@ _mysql_row_to_dict( goto error; } int err = PyDict_Contains(r, pyname); - if (res < 0) { + if (err < 0) { // error Py_DECREF(v); goto error; } - if (res) { + if (err) { // duplicate Py_DECREF(pyname); pyname = PyUnicode_FromFormat("%s.%s", fields[i].table, fields[i].name); if (pyname == NULL) { From f3ea4f58d873fe7511a6dbd7e0f49209d7ba5ebc Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 18:05:11 +0900 Subject: [PATCH 7/8] fix caching --- MySQLdb/_mysql.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index f8f81a58..27880ca2 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1375,7 +1375,7 @@ _mysql__fetch_row( _convertfunc *convert_row = row_converters[how]; PyObject *cache = NULL; - if (maxrows > 0 && how > 1) { + if (maxrows > 0 && how > 0) { cache = PyTuple_New(mysql_num_fields(self->result)); if (!cache) { return -1; From 3816a98bb6f21ccef5b428ae885f913fa8ff4b8c Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 8 Dec 2020 18:23:16 +0900 Subject: [PATCH 8/8] fix test --- tests/test_cursor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 5e88a9a2..91f0323e 100644 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -138,8 +138,9 @@ def test_dictcursor(): assert a is b # Old fetchtype + cursor._fetch_type = 2 cursor.execute("SELECT * FROM t1 JOIN t2 ON t1.b=t2.b") - rows = cursor._result.fetch_row(0, 2) + rows = cursor.fetchall() assert len(rows) == 2 assert rows[0] == {"t1.a": 1, "t1.b": 1, "t1.c": 47, "t2.b": 1, "t2.c": 1}