Skip to content
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

Unintentional changes in sqlite3.connect() #93044

Closed
serhiy-storchaka opened this issue May 21, 2022 · 3 comments · Fixed by #93046
Closed

Unintentional changes in sqlite3.connect() #93044

serhiy-storchaka opened this issue May 21, 2022 · 3 comments · Fixed by #93046
Labels
3.11 expert-sqlite3 type-bug

Comments

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented May 21, 2022

There are some unintentional consequences of converting sqlite3.connect() to Argument Clinic in #40956.

  1. The database argument always converted to bytes.
  2. Keyword arguments are passed as positional arguments to factory().
  3. If an argument is not passed to sqlite3.connect(), its default value is passed to factory().

It all works with the default factory=Connection, but factory can be an arbitrary callable, not completely compatible with Connection(). There may be a user code which uses a factory which only works with string database and does not support the uri argument. It worked fine in older versions when it is called with correct arguments, bet will become failing in 3.11.

This is a hypothetical scenario, but we should at least add a note about potential incompatibility. And I think that it is better to get rid of PyUnicode_FSConverter here.
@serhiy-storchaka serhiy-storchaka added type-bug 3.11 expert-sqlite3 labels May 21, 2022
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue May 21, 2022
@serhiy-storchaka
pythongh-93044: No longer convert the database argument of sqlite3.co…
19c77f5 
…nnect() to bytes

Just pass it to the factory as is.
@serhiy-storchaka serhiy-storchaka linked a pull request May 21, 2022 that will close this issue
gh-93044: No longer convert the database argument of sqlite3.connect() to bytes #93046
Merged
@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 21, 2022

FTR, sqlite3 was converted to Argument Clinic in gh-85128 (bpo-40956).

@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 21, 2022

Sounds good to me.

serhiy-storchaka added a commit that referenced this issue May 21, 2022
@serhiy-storchaka
gh-93044: No longer convert the database argument of sqlite3.connect(…
14c0d33 
…) to bytes (GH-93046)

Just pass it to the factory as is.
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue May 21, 2022
@serhiy-storchaka
[3.11] pythongh-93044: No longer convert the database argument of sql…
2caba36 
…ite3.connect() to bytes (pythonGH-93046)

Just pass it to the factory as is..
(cherry picked from commit 14c0d33)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this issue May 21, 2022
@serhiy-storchaka
[3.11] gh-93044: No longer convert the database argument of sqlite3.c…
d9a48d2 
…onnect() to bytes (GH-93046) (GH-93048)

Just pass it to the factory as is.
(cherry picked from commit 14c0d33)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 21, 2022

Resolved with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.11 expert-sqlite3 type-bug
Projects
None yet
Milestone
No milestone
2 participants
@serhiy-storchaka @erlend-aasland