Skip to content

Commit

Permalink
Fix PyArg_ParseTupleAndKeywords calls that accept optional strings.
Browse files Browse the repository at this point in the history
A lot of the keywords in Cursor methods are optional, but I incorrectly used 's' in the format
string.  This meant you could call the function without passing them, but would fail if you
passed None for them, which is not what I intended.

It has not been noticed in normal usage because it is easier to call `f('t1')` than
`f('t1', schema=None, catalog=None)`.  However, when you use a wrapper, like aioodbc does, it
will fail:

    def wrapper(table, schema=None, catalog=None):
        call(cursor.columns, table, schema=schema, catalog=catalog)

    def call(func, *args, **kwargs):
        func(*args, **kwargs)

At this point, kwargs is {schema:None, catalog:None} and now we're passing None explicitly.

The solution, of course, is to use 'z' instead.

Note that I have only corrected Cursor methods.  I only wrote a test for Cursor.columns in
SQL Server Python 3 and PostgreSQL Python 3.  This bug is in pyodbc before any database calls
are made so it is database-independent.
  • Loading branch information
mkfiserv authored and mkleehammer committed Oct 20, 2017
1 parent a60c6d5 commit 383fa6c
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 11 deletions.
18 changes: 10 additions & 8 deletions src/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ static PyObject* Cursor_tables(PyObject* self, PyObject* args, PyObject* kwargs)
const char* szTableName = 0;
const char* szTableType = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ssss", Cursor_tables_kwnames, &szTableName, &szCatalog, &szSchema, &szTableType))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zzzz", Cursor_tables_kwnames, &szTableName, &szCatalog, &szSchema, &szTableType))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down Expand Up @@ -1284,7 +1284,7 @@ static PyObject* Cursor_columns(PyObject* self, PyObject* args, PyObject* kwargs
const char* szTable = 0;
const char* szColumn = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ssss", Cursor_column_kwnames, &szTable, &szCatalog, &szSchema, &szColumn))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zzzz", Cursor_column_kwnames, &szTable, &szCatalog, &szSchema, &szColumn))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down Expand Up @@ -1355,7 +1355,7 @@ static PyObject* Cursor_statistics(PyObject* self, PyObject* args, PyObject* kwa
PyObject* pUnique = Py_False;
PyObject* pQuick = Py_True;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|ssOO", Cursor_statistics_kwnames, &szTable, &szCatalog, &szSchema,
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|zzOO", Cursor_statistics_kwnames, &szTable, &szCatalog, &szSchema,
&pUnique, &pQuick))
return 0;

Expand Down Expand Up @@ -1433,7 +1433,7 @@ static PyObject* _specialColumns(PyObject* self, PyObject* args, PyObject* kwarg
const char* szSchema = 0;
PyObject* pNullable = Py_True;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|ssO", Cursor_specialColumn_kwnames, &szTable, &szCatalog, &szSchema, &pNullable))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|zzO", Cursor_specialColumn_kwnames, &szTable, &szCatalog, &szSchema, &pNullable))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down Expand Up @@ -1504,7 +1504,9 @@ static PyObject* Cursor_primaryKeys(PyObject* self, PyObject* args, PyObject* kw
const char* szCatalog = 0;
const char* szSchema = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|ss", Cursor_primaryKeys_kwnames, &szTable, &szCatalog, &szSchema))
printf("ARGS: c=%d\n", (int)PyTuple_Size(args));

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|zz", Cursor_primaryKeys_kwnames, &szTable, &szCatalog, &szSchema))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down Expand Up @@ -1574,7 +1576,7 @@ static PyObject* Cursor_foreignKeys(PyObject* self, PyObject* args, PyObject* kw
const char* szForeignCatalog = 0;
const char* szForeignSchema = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ssssss", Cursor_foreignKeys_kwnames, &szTable, &szCatalog, &szSchema,
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zzzzzz", Cursor_foreignKeys_kwnames, &szTable, &szCatalog, &szSchema,
&szForeignTable, &szForeignCatalog, &szForeignSchema))
return 0;

Expand Down Expand Up @@ -1802,7 +1804,7 @@ static PyObject* Cursor_procedureColumns(PyObject* self, PyObject* args, PyObjec
const char* szCatalog = 0;
const char* szSchema = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|sss", Cursor_procedureColumns_kwnames, &szProcedure, &szCatalog, &szSchema))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zzz", Cursor_procedureColumns_kwnames, &szProcedure, &szCatalog, &szSchema))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down Expand Up @@ -1861,7 +1863,7 @@ static PyObject* Cursor_procedures(PyObject* self, PyObject* args, PyObject* kwa
const char* szCatalog = 0;
const char* szSchema = 0;

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|sss", Cursor_procedures_kwnames, &szProcedure, &szCatalog, &szSchema))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zzz", Cursor_procedures_kwnames, &szProcedure, &szCatalog, &szSchema))
return 0;

Cursor* cur = Cursor_Validate(self, CURSOR_REQUIRE_OPEN);
Expand Down
36 changes: 33 additions & 3 deletions tests3/pgtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def test_varchar_many(self):
v2 = '0123456789' * 30
v3 = '9876543210' * 30

self.cursor.execute("insert into t1(c1, c2, c3) values (?,?,?)", v1, v2, v3);
self.cursor.execute("insert into t1(c1, c2, c3) values (?,?,?)", v1, v2, v3)
row = self.cursor.execute("select c1, c2, c3 from t1").fetchone()

self.assertEqual(v1, row.c1)
Expand Down Expand Up @@ -472,7 +472,7 @@ def test_cnxn_execute_error(self):
self.failUnlessRaises(pyodbc.Error, self.cnxn.execute, "insert into t1 values (1)")

def test_row_repr(self):
self.cursor.execute("create table t1(a int, b int, c int, d int)");
self.cursor.execute("create table t1(a int, b int, c int, d int)")
self.cursor.execute("insert into t1 values(1,2,3,4)")

row = self.cursor.execute("select * from t1").fetchone()
Expand Down Expand Up @@ -517,6 +517,36 @@ def test_cnxn_set_attr(self):
SQL_MODE_READ_ONLY = 1
self.cnxn.set_attr(SQL_ATTR_ACCESS_MODE, SQL_MODE_READ_ONLY)

def test_columns(self):
# When using aiohttp, `await cursor.primaryKeys('t1')` was raising the error
#
# Error: TypeError: argument 2 must be str, not None
#
# I'm not sure why, but PyArg_ParseTupleAndKeywords fails if you use "|s" for an
# optional string keyword when calling indirectly.

self.cursor.execute("create table t1(a int, b varchar(3))")

self.cursor.columns('t1')
results = {row.column_name: row for row in self.cursor}
row = results['a']
assert row.type_name == 'int4', row.type_name
row = results['b']
assert row.type_name == 'varchar'
assert row.precision == 3, row.precision

# Now do the same, but specifically pass in None to one of the keywords. Old versions
# were parsing arguments incorrectly and would raise an error. (This crops up when
# calling indirectly like columns(*args, **kwargs) which aiodbc does.)

self.cursor.columns('t1', schema=None, catalog=None)
results = {row.column_name: row for row in self.cursor}
row = results['a']
assert row.type_name == 'int4', row.type_name
row = results['b']
assert row.type_name == 'varchar'
assert row.precision == 3


def main():
from optparse import OptionParser
Expand Down Expand Up @@ -559,7 +589,7 @@ def main():
s = unittest.TestSuite([ PGTestCase(connection_string, options.ansi, m) for m in methods ])

testRunner = unittest.TextTestRunner(verbosity=options.verbose)
result = testRunner.run(s)
testRunner.run(s)

if __name__ == '__main__':

Expand Down
30 changes: 30 additions & 0 deletions tests3/sqlservertests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,36 @@ def test_exc_integrity(self):
self.cursor.execute("insert into t1 values ('one')")
self.failUnlessRaises(pyodbc.IntegrityError, self.cursor.execute, "insert into t1 values ('one')")

def test_columns(self):
# When using aiohttp, `await cursor.primaryKeys('t1')` was raising the error
#
# Error: TypeError: argument 2 must be str, not None
#
# I'm not sure why, but PyArg_ParseTupleAndKeywords fails if you use "|s" for an
# optional string keyword when calling indirectly.

self.cursor.execute("create table t1(a int, b varchar(3))")

self.cursor.columns('t1')
results = {row.column_name: row for row in self.cursor}
row = results['a']
assert row.type_name == 'int', row.type_name
row = results['b']
assert row.type_name == 'varchar'
assert row.column_size == 3

# Now do the same, but specifically pass in None to one of the keywords. Old versions
# were parsing arguments incorrectly and would raise an error. (This crops up when
# calling indirectly like columns(*args, **kwargs) which aiodbc does.)

self.cursor.columns('t1', schema=None, catalog=None)
results = {row.column_name: row for row in self.cursor}
row = results['a']
assert row.type_name == 'int', row.type_name
row = results['b']
assert row.type_name == 'varchar'
assert row.column_size == 3


def main():
from optparse import OptionParser
Expand Down

0 comments on commit 383fa6c

Please sign in to comment.