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 added4 commits

August 9, 2021 23:15

bpo-44859: Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE

0c8a88d

If SQLITE_MISUSE is raised, it is a sqlite3 module bug. Users of thesqlite3 module are not responsible of using the SQLite C API correctly.

Don't overwrite BufferErrors when converting to BLOB

aa755d2

Raise ProgrammingError if user tries to execute() more than one state…

e51d6c1

…ment

Raise DataError if query contains null chars

452ce40

erlend-aasland requested a review fromberkerpeksag as acode owner

August 9, 2021 22:04

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

Aug 9, 2021

bedevere-bot added the awaiting review label

Aug 9, 2021

erlend-aasland requested a review fromserhiy-storchaka

August 9, 2021 22:04

Add NEWS

f9a5dc6

erlend-aasland force-pushed thesqlite-errors branch from608f9fe tof9a5dc6Compare

August 9, 2021 22:09

erlend-aasland closed this

Aug 10, 2021

erlend-aasland reopened this

Aug 10, 2021

Merge branch 'main' into sqlite-errors

b08d507

Copy link

ContributorAuthor

erlend-aasland commentedSep 10, 2021

@serhiy-storchaka would you mind reviewing this?

erlend-aasland mentioned this pull request

Sep 10, 2021

gh-89018: Improve documentation ofsqlite3 exceptions#27645

Merged

Erlend E. Aasland added3 commits

September 14, 2021 21:19

Merge branch 'main' into sqlite-errors

ddcab47

Merge branch 'main' into sqlite-errors

6d5361c

_pysqlite_set_result() MUST set an exception if it returns -1

f2300bd

Failure to do so, will result in an assertion failure / segfault.

Copy link

ContributorAuthor

erlend-aasland commentedSep 20, 2021

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

erlend-aasland requested a review frommalemburg

October 27, 2021 12:06

Copy link

ContributorAuthor

erlend-aasland commentedOct 27, 2021

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

Erlend E. Aasland added2 commits

November 1, 2021 11:03

Merge branch 'main' into sqlite-errors

84af2b9

Merge branch 'main' into sqlite-errors

d06b769

JelleZijlstra requested changes

Jan 25, 2022

View reviewed changes

Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst OutdatedShow resolvedHide resolved

Modules/_sqlite/connection.c OutdatedShow resolvedHide resolved

Modules/_sqlite/connection.cShow resolvedHide resolved

bedevere-bot added awaiting core review and removed awaiting review labels

Jan 25, 2022

Copy link

Member

JelleZijlstra commentedFeb 17, 2022

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

Copy link

ContributorAuthor

erlend-aasland commentedFeb 17, 2022

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 E. Aasland added2 commits

February 26, 2022 22:49

Merge branch 'main' into sqlite-errors

8eb783e

AdaptpythonGH-27642tests

d3186be

Address review: add tests for PyObject_GetBuffer failures

b4a95bd

Copy link

ContributorAuthor

erlend-aasland commentedFeb 26, 2022

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.

erlend-aasland added the DO-NOT-MERGE label

Feb 26, 2022

JelleZijlstra self-assigned this

Feb 26, 2022

Copy link

ContributorAuthor

erlend-aasland commentedMar 2, 2022•
edited
Loading

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 becauseb899126 produces a ton of warnings in the Django test suite. I revertedb899126 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):

RaiseInterfaceError iso.ProgrammingError forSQLITE_MISUSE. IfSQLITE_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.
RaiseProgrammingError iso.Warning if user tries toexecute() more than one statement
Yep, sounds fine.
RaiseDataError iso.ValueError if query containsNULL 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.