As a nice side effect, we can now remove the collation dict from the
connection object, since SQLite now takes care of destroying the
callback context.

This change is needed in order to make it more convenient to add a
module state to the sqlite3 module.

https://bugs.python.org/issue42064

FYI, the test suite already exercise clearing a collation callback and re-registering a collation callback.

BTW, I'd like to change the finally label at the end of pysqlite_connection_init_impl to error, for consistency with the rest of the code base.

Member

encukou left a comment

I see that the beginning of the function does some busywork converting name to uppercase and checking for allowed characters. Is that limitation there only to make that sure comparison of keys in the collations dict match sqlite3_strnicmp? If so, PyUnicode_AsUTF8(name) could be used instead.

Yes, I noticed that part as well. I haven't investigated why this was added in the first place. I'll try with PyUnicode_AsUTF8(). Thanks!

As for the finally→error rename, I'd say finally is a better name when it's also executed in the non-error case...

True.

FYI, the collation code (including the uppercase conversion / check), was added to pysqlite in 2006, when it still was an external project. The code seems to have been borrowed directly from apsw.

UPDATE:
SQLite accepts UTF8 collation names; there should be no reason to limit collation names to ASCII. Maybe there was a bug or a limitation in earlier SQLite versions. The apsw code has removed this limitation and now accepts UTF8 names. We can do that as well. I'll open up an issue for that.

UPDATE AGAIN:
I've created issue 44688. I'll open a PR once this is merged. PR #27395 opened.

This implies that SQLite now takes care of destroying the callback
context (the PyObject callable it has been passed), so we can strip the
collation dict from the connection object.