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 — bpo-44859: Raise more accurate exceptions in `sqlite3` by erlend-aasland · Pull Request #27695 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Aug 9, 2021 •
edited
Loading

Uh oh!

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Erlend E. Aasland added 4 commits August 9, 2021 23:15
Erlend E. Aasland
bpo-44859: Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE
0c8a88d 
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.
Erlend E. Aasland
Don't overwrite BufferErrors when converting to BLOB
aa755d2
Erlend E. Aasland
Raise ProgrammingError if user tries to execute() more than one state…
e51d6c1 
…ment
Erlend E. Aasland
Raise DataError if query contains null chars
452ce40
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner August 9, 2021 22:04
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Aug 9, 2021
@bedevere-bot bedevere-bot added the awaiting review label Aug 9, 2021
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka August 9, 2021 22:04
Erlend E. Aasland
Add NEWS
f9a5dc6
@erlend-aasland
erlend-aasland force-pushed the sqlite-errors branch from 608f9fe to f9a5dc6 Compare August 9, 2021 22:09
@erlend-aasland erlend-aasland closed this Aug 10, 2021
@erlend-aasland erlend-aasland reopened this Aug 10, 2021
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
b08d507
@erlend-aasland

erlend-aasland commented Sep 10, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka would you mind reviewing this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland mentioned this pull request Sep 10, 2021
gh-89018: Improve documentation of sqlite3 exceptions #27645
Merged
Erlend E. Aasland added 3 commits September 14, 2021 21:19
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
ddcab47
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
6d5361c
Erlend E. Aasland
_pysqlite_set_result() MUST set an exception if it returns -1
f2300bd 
Failure to do so, will result in an assertion failure / segfault.
@erlend-aasland

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
erlend-aasland requested a review from malemburg October 27, 2021 12:06
@erlend-aasland

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland added 2 commits November 1, 2021 11:03
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
84af2b9
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
d06b769
JelleZijlstra
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 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Jan 25, 2022
@JelleZijlstra

JelleZijlstra commented Feb 17, 2022

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland added 2 commits February 26, 2022 22:49
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
8eb783e
Erlend E. Aasland
Adapt pythonGH-27642 tests
d3186be
Erlend E. Aasland
Address review: add tests for PyObject_GetBuffer failures
b4a95bd
@erlend-aasland

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland added the DO-NOT-MERGE label Feb 26, 2022
@JelleZijlstra JelleZijlstra self-assigned this Feb 26, 2022
@erlend-aasland

erlend-aasland commented Mar 2, 2022 •
edited
Loading

Uh oh!

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland added 2 commits March 8, 2022 09:59
Erlend E. Aasland
Address reviews
3792cf0
Erlend E. Aasland
Merge branch 'main' into sqlite-errors
99c9040
@erlend-aasland erlend-aasland removed the DO-NOT-MERGE label Mar 8, 2022
@erlend-aasland
erlend-aasland requested a review from JelleZijlstra March 8, 2022 09:24
JelleZijlstra
JelleZijlstra reviewed Mar 8, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra
JelleZijlstra approved these changes Mar 17, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Mar 17, 2022
@JelleZijlstra
JelleZijlstra merged commit 4674fd4 into python:main Mar 17, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 17, 2022
@erlend-aasland
erlend-aasland deleted the sqlite-errors branch March 17, 2022 08:16
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 16, 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 @JelleZijlstra @the-knights-who-say-ni @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.