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

gh-89022: Improve sqlite3 exceptions related to binding params and API misuse #91572

Merged
merged 9 commits into from
May 4, 2022

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 15, 2022

  • Map SQLITE_MISUSE to InterfaceError instead of ProgrammingError
  • Raise more accurate exceptions when binding parameters fail

Fixes gh-89022

SQLITE_MISUSE implies misuse of the SQLite C API, which, if it happens,
is _not_ a user error; it is an sqlite3 extension module error.
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Apr 15, 2022

See also https://sqlite.org/rescode.html#misuse:

If SQLite ever returns SQLITE_MISUSE from any interface, that means that the application is incorrectly coded and needs to be fixed. Do not ship an application that sometimes returns SQLITE_MISUSE from a standard SQLite interface because that application contains potentially serious bugs.

Instead of always raising InterfaceError, guessing what went wrong,
raise accurate exceptions with more accurate error messages.
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented May 2, 2022

Example of snippets with improved messages; this PR compared to latest official 3.11 release.

Limiting the length of string literals:

import sqlite3
cx = sqlite3.connect(":memory:")
cx.set_trace_callback(print)
cx.setlimit(sqlite3.SQLITE_LIMIT_LENGTH, 7)
cx.execute("select ?", ("a"*10,))
$ python3.11 lim.py 
Traceback (most recent call last):
  File "/Users/erlendaasland/src/cpython-build/lim.py", line 5, in <module>
    cx.execute("select ?", ("a"*10,))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.
$ ./python.exe lim.py 
Traceback (most recent call last):
  File "/Users/erlendaasland/src/cpython-build/lim.py", line 5, in <module>
    cx.execute("select ?", ("a"*10,))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.DataError: string or blob too big

Trying to bind to an unsupported type (note, SQLite parameters are 1-indexed, not 0-indexed):

import sqlite3
cx = sqlite3.connect(":memory:")
val = dict()
cx.execute("select ?", (val,))
$ python3.11 type.py 
Traceback (most recent call last):
  File "/Users/erlendaasland/src/cpython-build/type.py", line 4, in <module>
    cx.execute("select ?", (val,))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.
$ ./python.exe type.py 
Traceback (most recent call last):
  File "/Users/erlendaasland/src/cpython-build/type.py", line 4, in <module>
    cx.execute("select ?", (val,))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.ProgrammingError: Error binding parameter 1: type 'dict' in not supported

@erlend-aasland erlend-aasland changed the title gh-89022: Map SQLITE_MISUSE to sqlite3.InterfaceError gh-89022: Improve sqlite3 exceptions related to binding params and API misuse May 2, 2022
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me when comparing with https://peps.python.org/pep-0249/#exceptions

@malemburg
Copy link
Member

Hi Erlend, the explicit exception changes look fine to me, since anything that a programmer using the module can potentially do wrong in terms of e.g. passing wrong number of parameters, using binding parameters where they are not allowed, etc. should result in a ProgrammingError.

I'm not sure about the SQLITE_MISUSE, though, since the SQLite docs are rather unspecific on what exactly can cause such an error. The one example they give (using a prepared statement after that prepared statement has been finalized) does point to an error which originates in the API itself rather than an error caused by the programmer using the API, so InterfaceError sounds correct.

In mxODBC, I use InterfaceError for e.g. using invalid attribute values passed to an ODBC API, or invalid lengths, options, invalid cursor state, etc. I.e. things where mxODBC itself would be doing something wrong, not necessarily the programmer. Sometimes the distinction is not a very clear one, though, so I wouldn't call the mapping I'm using the only correct one.

Copy link
Member

@malemburg malemburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo the comments I made on the PR.

@malemburg
Copy link
Member

LGTM, modulo the comments I made on the PR.

Strange, I clicked on the "Add your review" button and approved the changes, but the bar at the top of the PR still says that Github is waiting for a review.

@erlend-aasland
Copy link
Contributor Author

I'm not sure about the SQLITE_MISUSE, though, since the SQLite docs are rather unspecific on what exactly can cause such an error. The one example they give (using a prepared statement after that prepared statement has been finalized) does point to an error which originates in the API itself rather than an error caused by the programmer using the API, so InterfaceError sounds correct.

Yes, the example you point to is definitely an InterfaceError. Also note this sentence, quoted from the SQLITE_MISUSE docs:

If SQLite ever returns SQLITE_MISUSE from any interface, that means that the application is incorrectly coded and needs to be fixed. Do not ship an application that sometimes returns SQLITE_MISUSE from a standard SQLite interface because that application contains potentially serious bugs.

I think this justifies mapping SQLITE_MISUSE to InterfaceError.

In mxODBC, I use InterfaceError for e.g. using invalid attribute values passed to an ODBC API, or invalid lengths, options, invalid cursor state, etc. I.e. things where mxODBC itself would be doing something wrong, not necessarily the programmer.

Exactly; this is also my understanding of InterfaceError.

Thanks for reviewing, and thanks for your input; highly appreciated.

@erlend-aasland
Copy link
Contributor Author

Strange, I clicked on the "Add your review" button and approved the changes, but the bar at the top of the PR still says that Github is waiting for a review.

Looks like GitHub forked you! :)
Screenshot 2022-05-03 at 13 46 17

@erlend-aasland
Copy link
Contributor Author

FYI, I've resolved the conflicting files, so this is ready to land when the CI finishes.

@JelleZijlstra JelleZijlstra self-assigned this May 3, 2022
@JelleZijlstra
Copy link
Member

Windows x64 was stuck, I restarted it.

@JelleZijlstra JelleZijlstra merged commit 090819e into python:main May 4, 2022
@erlend-aasland erlend-aasland deleted the sqlite-misuse branch May 4, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve some sqlite3 errors
4 participants