Skip to content

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

Merged
pablogsal merged 23 commits intopython:mainfrom
erlend-aasland:sqlite-extended-error-codes
Nov 2, 2021
Merged

bpo-24139: Add support for SQLite extended result codes#28076
pablogsal merged 23 commits intopython:mainfrom
erlend-aasland:sqlite-extended-error-codes

Conversation

@erlend-aasland
Copy link
Copy Markdown
Contributor

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

@erlend-aasland
Copy link
Copy Markdown
Contributor Author

erlend-aasland commented Aug 30, 2021

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

erlend-aasland commented Aug 31, 2021

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

bedevere-bot commented Sep 1, 2021

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

pablogsal
pablogsal reviewed Sep 19, 2021
View reviewed changes
Comment thread Modules/_sqlite/module.c
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
Copy Markdown
Contributor Author

erlend-aasland commented Oct 19, 2021

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