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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-44688: Remove ASCII limitation from `sqlite3` collation names #27395

Merged

encukou merged 7 commits into python:main from erlend-aasland:sqlite-remove-collation-ascii-limitation

Jul 29, 2021

Contributor

erlend-aasland commented Jul 27, 2021 •

edited by bedevere-bot

https://bugs.python.org/issue44688

erlend-aasland added 2 commits


          bpo-44688: sqlite3 now accepts UTF8 collation names

3f9e3c7


          Docs

13e17db

erlend-aasland requested a review from berkerpeksag as a code owner

bedevere-bot added the awaiting review label

the-knights-who-say-ni added the CLA signed label

erlend-aasland requested a review from encukou

Contributor Author

erlend-aasland commented Jul 27, 2021 •

edited

cc. @encukou: ref. discussion in #27156 (comment)

erlend-aasland mentioned this pull request

bpo-42064: Migrate to sqlite3_create_collation_v2 #27156

Merged

encukou reviewed

View changes

Member

encukou left a comment

Nice! Just a terminology issue: the Python interface doesn't deal with UTF-8 (bytes); in py3 it always took a Unicode string.

For completeness, could you mention the pre-3.11 limitation in the documentation? (I think it's OK to only have it in the 3.11+ docs, but IMO it should be there for people porting code from 3.11 to previous versions.)

Misc/NEWS.d/next/Library/2021-07-20-23-28-26.bpo-44688.buFgz3.rst Outdated Show resolved Hide resolved

Lib/sqlite3/test/hooks.py Outdated Show resolved Hide resolved

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved

erlend-aasland and others added 3 commits


          Address review: adjust NEWS entry wording

ea65d23

Co-authored-by: Petr Viktorin <encukou@gmail.com>


          Address review: revert test method renaming

f34461d

Co-authored-by: Petr Viktorin <encukou@gmail.com>


          Address review: Improve What's New wording

1f0e959

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Contributor Author

erlend-aasland commented Jul 28, 2021

Nice! Just a terminology issue: the Python interface doesn't deal with UTF-8 (bytes); in py3 it always took a Unicode string.

Thanks for the heads up and the suggested improvements.

For completeness, could you mention the pre-3.11 limitation in the documentation? (I think it's OK to only have it in the 3.11+ docs, but IMO it should be there for people porting code from 3.11 to previous versions.)

Of course.


          Address review: mention previous limitation in the docs

74f7108

erlend-aasland requested a review from encukou


          Remove trailing whitespace

dfa343f

Contributor Author

erlend-aasland commented Jul 28, 2021

The address sanitiser test timed out during one of the multiprocessing tests. Looks unrelated.

https://github.com/python/cpython/pull/27395/checks?check_run_id=3181419480

Member

encukou commented Jul 28, 2021

I see @pablogsal added the address sanitizer in #26640.
Is it normally flaky?

encukou closed this

encukou reopened this

Member

pablogsal commented Jul 28, 2021

I see @pablogsal added the address sanitizer in #26640.
Is it normally flaky?

No regarding detection of problems but a small subset of times it times out. What error are you experiencing?

Contributor Author

erlend-aasland commented Jul 28, 2021

Is it normally flaky?

No regarding detection of problems but a small subset of times it times out. What error are you experiencing?

test_get in test_multiprocessing_fork timed out in both test runs on the last CI run. I can't recall seeing that particular failure before. See attached log.

encukou merged commit 5269c09 into python:main

21 of 22 checks passed

bedevere-bot removed the awaiting review label

erlend-aasland deleted the sqlite-remove-collation-ascii-limitation branch

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue


          Merge branch 'main' into sqlite-rollback

f5d5e68

Sync with main bco. pythonGH-27395

JuniorJPDJ added a commit to JuniorJPDJ/cpython that referenced this issue


          bpo-44688: Remove ASCII limitation from sqlite3 collation names (py…

ab17f82

…thonGH-27395)

erlend-aasland mentioned this pull request

Drop _Py_IDENTIFIER API usage coleifer/pysqlite3#31

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment