The Wayback Machine - https://web.archive.org/web/20220203063946/https://github.com/python/cpython/pull/27695
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

Jump to bottom

bpo-44859: Raise more accurate exceptions in sqlite3 #27695

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

bpo-44859: Raise more accurate exceptions in sqlite3 #27695

wants to merge 11 commits into from
+32 −13

Conversation

@erlend-aasland
Copy link
Contributor

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

Improve exception compliancy with PEP 249

  • 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.
  • Don't overwrite BufferError with ValueError when converting to BLOB
  • Raise ProgrammingError iso. Warning if user tries to execute() more
    than one statement
  • Raise DataError iso. ValueError if query contains NULL characters
  • Make sure _pysqlite_set_result raises an exception if it returns -1

https://bugs.python.org/issue44859
@erlend-aasland
Copy link
Contributor Author

@erlend-aasland erlend-aasland commented Sep 10, 2021

@serhiy-storchaka would you mind reviewing this?

@erlend-aasland erlend-aasland mentioned this pull request Sep 10, 2021
Draft
@erlend-aasland
Copy link
Contributor Author

@erlend-aasland erlend-aasland commented Sep 20, 2021

@serhiy-storchaka PTAL. I just fixed bug in _pysqlite_set_result that could result in a segfault.

@erlend-aasland erlend-aasland requested a review from malemburg Oct 27, 2021
@erlend-aasland
Copy link
Contributor Author

@erlend-aasland erlend-aasland commented Oct 27, 2021

@malemburg would you mind taking a look at this PR? I'm slowly trying to align the exceptions raised with PEP 249.

JelleZijlstra
JelleZijlstra suggested changes Jan 25, 2022
View changes
Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Show resolved Hide resolved
Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst Show resolved Hide resolved
Modules/_sqlite/connection.c Show resolved Hide resolved
Modules/_sqlite/connection.c
@@ -556,8 +556,6 @@ _pysqlite_set_result(sqlite3_context* context, PyObject* py_val)
} else if (PyObject_CheckBuffer(py_val)) {
Py_buffer view;
if (PyObject_GetBuffer(py_val, &view, PyBUF_SIMPLE) != 0) {
PyErr_SetString(PyExc_ValueError,
Copy link
Member

@JelleZijlstra JelleZijlstra Jan 25, 2022

Choose a reason for hiding this comment

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

Looks like there's no test for this change.

Copy link
Contributor Author

@erlend-aasland erlend-aasland Jan 25, 2022

Choose a reason for hiding this comment

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

I'll add one, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra

@berkerpeksag berkerpeksag

@serhiy-storchaka serhiy-storchaka

@malemburg malemburg

Assignees
No one assigned
Labels
awaiting core review CLA signed
Projects
None yet
Milestone
No milestone
4 participants
@erlend-aasland @JelleZijlstra @the-knights-who-say-ni @bedevere-bot