GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-89022: Improve sqlite3 exceptions related to binding params and API misuse by erlend-aasland · Pull Request #91572 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Apr 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Fixes gh-89022

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Erlend E. Aasland
pythongh-89022: Map SQLITE_MISUSE to sqlite3.InterfaceError
2868daf
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
erlend-aasland requested a review from berkerpeksag as a code owner April 15, 2022 15:02
@bedevere-bot bedevere-bot added the awaiting review label Apr 15, 2022
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka April 15, 2022 15:03
@erlend-aasland

erlend-aasland commented Apr 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend E. Aasland
Add NEWS
cfba333
@erlend-aasland erlend-aasland mentioned this pull request Apr 15, 2022
gh-69093: improve sqlite3.Connection.blobopen() error handling #91571
Merged
@erlend-aasland
erlend-aasland requested review from malemburg May 1, 2022 23:26
erlend-aasland added 5 commits May 1, 2022 20:41
@erlend-aasland
Raise better errors when binding parameters fail.
c32e2e6
Instead of always raising InterfaceError, guessing what went wrong,
raise accurate exceptions with more accurate error messages.
@erlend-aasland
Expand NEWS entry
268e4e3
@erlend-aasland
Sync with main
d7962c2
@erlend-aasland
Improve more bind exceptions
c83172a
@erlend-aasland
pysqlite_statement_bind_parameter is local to statement.c, so make it…
76f7ed1
… static
@erlend-aasland

erlend-aasland commented May 2, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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
JelleZijlstra
JelleZijlstra approved these changes May 2, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels May 2, 2022
@malemburg

malemburg commented May 3, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

malemburg
malemburg approved these changes May 3, 2022
View reviewed changes

@malemburg malemburg left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@malemburg

malemburg commented May 3, 2022

Copy link
Copy Markdown
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 reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented May 3, 2022

Copy link
Copy Markdown
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.

malemburg reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented May 3, 2022

Copy link
Copy Markdown
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

malemburg reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland added 2 commits May 3, 2022 15:58
@erlend-aasland
Merge branch 'main' into sqlite-misuse
6ae7bde
@erlend-aasland
Merge branch 'main' into sqlite-misuse
5051708
@erlend-aasland

erlend-aasland commented May 3, 2022

Copy link
Copy Markdown
Contributor Author

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

JelleZijlstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra JelleZijlstra self-assigned this May 3, 2022
@JelleZijlstra

JelleZijlstra commented May 4, 2022

Copy link
Copy Markdown
Member

Windows x64 was stuck, I restarted it.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
JelleZijlstra merged commit 090819e into python:main May 4, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label May 4, 2022
@erlend-aasland
erlend-aasland deleted the sqlite-misuse branch May 4, 2022 14:38
@erlend-aasland erlend-aasland mentioned this pull request May 18, 2022
Improve some sqlite3 errors #89022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@JelleZijlstra JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve some sqlite3 errors

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@erlend-aasland @malemburg @JelleZijlstra @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.