Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
bpo-42064: Migrate tosqlite3_create_collation_v2#27156
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
bpo-42064: Migrate tosqlite3_create_collation_v2#27156
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This implies that SQLite now takes care of destroying the callbackcontext (the PyObject callable it has been passed), so we can strip thecollation dict from the connection object.
erlend-aasland commentedJul 15, 2021
|
erlend-aasland commentedJul 15, 2021
FYI, the test suite already exercise clearing a collation callback and re-registering a collation callback. |
erlend-aasland commentedJul 15, 2021
BTW, I'd like to change the |
encukou left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good so far!
I see that the beginning of the function does some busywork convertingname to uppercase and checking for allowed characters. Is that limitation there only to make that sure comparison of keys in thecollations dict matchsqlite3_strnicmp? If so,PyUnicode_AsUTF8(name) could be used instead.
As for thefinally→error rename, I'd sayfinally is a better name when it's also executed in the non-error case...
Uh oh!
There was an error while loading.Please reload this page.
erlend-aasland commentedJul 20, 2021
Yes, I noticed that part as well. I haven't investigated why this was added in the first place. I'll try with
True. |
erlend-aasland commentedJul 20, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
FYI, the collation code (including the uppercase conversion / check), was added to UPDATE: UPDATE AGAIN: |
encukou commentedJul 27, 2021
Thank you! |
Sync with main afterpythonGH-27156.
Uh oh!
There was an error while loading.Please reload this page.
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
sqlite3module.https://bugs.python.org/issue42064