FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-44859: Raise more accurate exceptions in `sqlite3` by erlend-aasland · Pull Request #27695 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-44859: Raise more accurate exceptions in sqlite3 - #27695

Merged
JelleZijlstra merged 16 commits into
python:mainfrom
erlend-aasland:sqlite-errors
Mar 17, 2022
ConversationCommits (16)ChecksFiles changed
Merged

bpo-44859: Raise more accurate exceptions in sqlite3#27695
JelleZijlstra merged 16 commits into
python:mainfrom
erlend-aasland:sqlite-errors

Conversation

erlend-aasland commented Aug 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Improve exception compliancy with PEP 249

  • Raise InterfaceError instead of ProgrammingError for SQLITE_MISUSE.
    If SQLITE_MISUSE is raised, it is a sqlite3 module bug. Users of the
    sqlite3 module are not responsible of using the SQLite C API correctly.
  • Don't overwrite BufferError with ValueError when conversion to BLOB fails.
  • Raise ProgrammingError instead of Warning if user tries to execute() more
    than one SQL statement.
  • Raise ProgrammingError instead of ValueError if an SQL query contains null characters.
  • Make sure _pysqlite_set_result raises an exception if it returns -1.

Fixes #89022

Erlend E. Aasland added 4 commits August 9, 2021 23:15

erlend-aasland commented Sep 10, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka would you mind reviewing this?

erlend-aasland mentioned this pull request Sep 10, 2021
Merged

erlend-aasland commented Sep 20, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka PTAL. I just fixed bug in _pysqlite_set_result that could result in a segfault.

erlend-aasland commented Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

@malemburg would you mind taking a look at this PR? I'm slowly trying to align the exceptions raised with PEP 249.

JelleZijlstra requested changes Jan 25, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Outdated
Comment thread Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Outdated
Comment thread Modules/_sqlite/connection.c Outdated
Comment thread Modules/_sqlite/connection.c

JelleZijlstra commented Feb 17, 2022

Copy link
Copy Markdown
Member

Looks like you didn't push the changes I previously asked for.

erlend-aasland commented Feb 17, 2022

Copy link
Copy Markdown
Contributor Author

Looks like you didn't push the changes I previously asked for.

Actually, it seems I forgot about this; I didn't make any changes yet :) I'll see if I can get to it any time soon. Thanks for the reminder!

erlend-aasland commented Feb 26, 2022

Copy link
Copy Markdown
Contributor Author

I'd like to run this through the Django test suite before eventually merging. Also, I'll have a talk with MAL regarding this change.

JelleZijlstra self-assigned this Feb 26, 2022

erlend-aasland commented Mar 2, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I'd like to run this through the Django test suite before eventually merging. Also, I'll have a talk with MAL regarding this change.

I finally got to run this through the Django test suite, and it works all fine; one down, one to go.

Observations while running the Django test suite

I got sidetracked with a long bisect operation because I was using SQLite 3.39.0 (development version), and because b899126 produces a ton of warnings in the Django test suite. I reverted b899126 and compiled strictly against SQLite 3.36.0 (macOS bundled), and all was ok.

UPDATE: I had a chat the other day with @malemburg, to get his comments regarding three specific DB-API related changes in this PR. I take the liberty to quote him here (hope that's ok, Marc-Andre):

  1. Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE. If SQLITE_MISUSE is raised, it is a sqlite3 module bug. Users of the sqlite3 module are not responsible of using the SQLite C API correctly.

    This sounds fine. InterfaceError is meant for module related errors.

  2. Raise ProgrammingError iso.Warning if user tries to execute() more than one statement

    Yep, sounds fine.

  3. Raise DataError iso. ValueError if query contains NULL characters

    Since queries are passed in by the programmer, I would use ProgrammingError here. DataError would be appropriate for errors related to e.g. wrong values passed in as bound parameters.

I will update the PR with the suggested change.

JelleZijlstra merged commit 4674fd4 into python:main Mar 17, 2022
erlend-aasland deleted the sqlite-errors branch March 17, 2022 08:16
serhiy-storchaka mentioned this pull request May 16, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra approved these changes
berkerpeksag Awaiting requested review from berkerpeksag
serhiy-storchaka Awaiting requested review from serhiy-storchaka
malemburg Awaiting requested review from malemburg

Assignees

JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve some sqlite3 errors

4 participants

Back | FazBrowse Home | New Git URL