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.

Sign up for GitHub

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

Jump to bottom

bpo-24139: Add support for SQLite extended result codes #28076

Merged
merged 23 commits into from
Nov 2, 2021
Merged

bpo-24139: Add support for SQLite extended result codes #28076

merged 23 commits into from
Nov 2, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 30, 2021 •
edited by bedevere-bot
Loading

@erlend-aasland
Copy link
Contributor Author

This PR uses a different approach than Aviv's proposed patch in bpo. I use sqlite3_errcode() to fetch the exception class, and sqlite3_extended_errcode() to set the exception attributes. IMO, this is clearer and more future proof than using a mask in get_exception_class().

@erlend-aasland erlend-aasland force-pushed the sqlite-extended-error-codes branch from a4daae8 to 0275ffd Compare August 30, 2021 21:46
@erlend-aasland erlend-aasland marked this pull request as draft August 30, 2021 22:28
@dimaqq
Copy link
Contributor

dimaqq commented Aug 31, 2021

from the tests:

Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/sqlite3/test/dbapi.py", line 276, in test_error_code_on_exception
    err_code = SQLITE_CANTOPEN
               ^^^^^^^^^^^^^^^
NameError: name 'SQLITE_CANTOPEN' is not defined

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Aug 31, 2021 •
edited
Loading

@dimaqq:

NameError: name 'SQLITE_CANTOPEN' is not defined

Yes, SQLITE_CANTOPEN_ISDIR is returned on Windows and if forgot to prefix it with sqlite.. But, I need to revisit the extended result code ifdefs anyway, so I've marked this PR as draft for now.

@erlend-aasland erlend-aasland marked this pull request as ready for review August 31, 2021 13:07
@erlend-aasland
Copy link
Contributor Author

PTAL, @dimaqq :)

@erlend-aasland erlend-aasland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 63347d8 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2021
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Sep 8, 2021 •
edited
Loading

The AMD64 Arch Linux Asan Debug PR buildbot failure is unrelated. See python/buildmaster-config#275

Erlend E. Aasland added 8 commits September 20, 2021 13:08
Add links to SQLite release docs
8e6c58c
Fix #if's for error codes between SQLite 3.8.2 and 3.8.10
a36a6b9 
SQLITE_CONSTRAINT_ROWID, SQLITE_READONLY_DBMOVED, and SQLITE_AUTH_USER
Add support for the SQLITE_RANGE primary result code
59da713 
The SQLITE_RANGE error indices that the parameter number argument to one
of the sqlite3_bind routines or the column number in one of the
sqlite3_column routines is out of range.  This should be considered a
misuse of the SQLite API, so we translate it to InterfaceError, as
defined by PEP 249.
Fix test_extended_error_code_on_exception version requirement
f6a9bbe
What's New
933f57b
Use managed connect in the added test
631e4d5
Merge branch 'main' into sqlite-extended-error-codes
54b1ceb
Improve comment
26517fc
@erlend-aasland
Copy link
Contributor Author

Let me know if you have further remarks, @pablogsal :)

@pablogsal pablogsal merged commit 456e27a into python:main Nov 2, 2021
@erlend-aasland erlend-aasland deleted the sqlite-extended-error-codes branch November 2, 2021 23:50
@bedevere-bot

This comment has been minimized.

Sign in to view
@erlend-aasland erlend-aasland mentioned this pull request Nov 3, 2021
Merged
@The-Compiler The-Compiler mentioned this pull request May 23, 2022
Closed
@eric-eclecticiq eric-eclecticiq mentioned this pull request Dec 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag berkerpeksag is a code owner

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@erlend-aasland @dimaqq @bedevere-bot @pablogsal @the-knights-who-say-ni