Skip to content

gh-108558: Add a couple more sqlite Row.keys() tests #108628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

gh-108558: Add a couple more sqlite Row.keys() tests #108628

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f194f84
Add a couple tests
EddInSverige Aug 29, 2023
0bce0d7
Add additional assertion that row objects are the same
EddInSverige Aug 29, 2023
fd7c46e
Appease the linting overlords
EddInSverige Aug 29, 2023
f07edbd
Refactor test into multiple tests
EddInSverige Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion Lib/test/test_sqlite3/test_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,33 @@ def test_sqlite_row_as_sequence(self):
self.assertEqual(list(reversed(row)), list(reversed(as_tuple)))
self.assertIsInstance(row, Sequence)

def test_sqlite_row_keys(self):
def test_sqlite_row_keys_return_columns_as_strings(self):
# Checks if the row object can return a list of columns as strings.
row = self.con.execute("select 1 as a, 2 as b").fetchone()
self.assertEqual(row.keys(), ['a', 'b'])

def test_sqlite_row_keys_raises_exception_on_empty_sql_query(self):
# Test exception raised on an empty sql query result
with self.assertRaises(AttributeError):
row = self.con.execute("select 1 as a where a == 'THISDOESNOTEXIST'").fetchone()
row.keys()
Comment on lines +256 to +258
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you narrow this down? Where is the AttributeError raised? Consider also using assertRaisesRegex.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the type of row?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So .fetchone() is returning None as per the comment below,

    # Returns either a row (as created by the row_factory) or None, but
    # putting None in the return annotation causes annoying false positives.
    def fetchone(self) -> Any: ...

and the type of keys has to be a list of strings,

class Row:
    def __init__(self, __cursor: Cursor, __data: tuple[Any, ...]) -> None: ...
    def keys(self) -> list[str]: ...

and here is the C code for the keys method,

static PyObject *
pysqlite_row_keys_impl(pysqlite_Row *self)
/*[clinic end generated code: output=efe3dfb3af6edc07 input=7549a122827c5563]*/
{
    PyObject* list;
    Py_ssize_t nitems, i;

    list = PyList_New(0);
    if (!list) {
        return NULL;
    }
    nitems = PyTuple_Size(self->description);

    for (i = 0; i < nitems; i++) {
        if (PyList_Append(list, PyTuple_GET_ITEM(PyTuple_GET_ITEM(self->description, i), 0)) != 0) {
            Py_DECREF(list);
            return NULL;
        }
    }

    return list;
}

however I'm not familiar with the codebase enough to go between the C code and the Python code to find where the error is being raised :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the query fails, no rows are returned, so fetchone returns None, so what you are really testing here is that None does not have a keys attribute :) I think we can do without this test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the type of row?

None :) As already mentioned, we don't need this test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If fetchone() returns not a Row, this code has no any relation to testing Row.keys().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #108628 (comment), Serhiy.


def test_sqlite_row_keys_returns_first_value_from_cursor_description(self):
# docs.python.org: "Immediately after a query, it is
# the first member of each tuple in Cursor.description."
cur = self.con.cursor(factory=MyCursor)
cur.execute("select 1 as a")
row = cur.fetchone()
Comment on lines +264 to +265
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR, it is also possible to do:

Suggested change
cur.execute("select 1 as a")
row = cur.fetchone()
row ,= cur.execute("select 1 as a")

and:

Suggested change
cur.execute("select 1 as a")
row = cur.fetchone()
[row] = cur.execute("select 1 as a")

... though they are pretty cryptic 😄

self.assertEqual(cur.description[0][0], row.keys()[0])

def test_sqlite_row_keys_are_equal_if_rows_are_equal(self):
# Two Row objects compare equal if they have identical column names
# and values. Assert the keys values are the same too.
row_1 = self.con.execute("select 1 as a, 2 as b").fetchone()
row_2 = self.con.execute("select 1 as a, 2 as b").fetchone()
self.assertEqual(row_1, row_2)
self.assertEqual(row_1.keys(), row_2.keys())
Comment on lines +268 to +274
Copy link
Contributor

@erlend-aasland erlend-aasland Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you object to this test as well, @serhiy-storchaka?

I think it would be simplified as such, though:

Suggested change
def test_sqlite_row_keys_are_equal_if_rows_are_equal(self):
# Two Row objects compare equal if they have identical column names
# and values. Assert the keys values are the same too.
row_1 = self.con.execute("select 1 as a, 2 as b").fetchone()
row_2 = self.con.execute("select 1 as a, 2 as b").fetchone()
self.assertEqual(row_1, row_2)
self.assertEqual(row_1.keys(), row_2.keys())
def test_sqlite_row_equality(self):
# Two Row objects compare equal if they have identical column names
# and values.
row_1 = self.con.execute("select 1 as a, 2 as b").fetchone()
row_2 = self.con.execute("select 1 as a, 2 as b").fetchone()
self.assertEqual(row_1, row_2)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH, this is already tested by test_sqlite_row_hash_cmp, so it is redundant.


def test_fake_cursor_class(self):
# Issue #24257: Incorrect use of PyObject_IsInstance() caused
# segmentation fault.
Expand Down