← 返回首页
bpo-44859: Improve error handling in sqlite3 and change some errors by serhiy-storchaka · Pull Request #27654 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

bpo-44859: Improve error handling in sqlite3 and change some errors#27654

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:sqlite-errors
Aug 8, 2021
Merged

bpo-44859: Improve error handling in sqlite3 and change some errors#27654
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:sqlite-errors

Conversation

serhiy-storchaka commented Aug 7, 2021
edited
Loading

Copy link
Copy Markdown
Member
  • MemoryError is now raised instead of sqlite3.Warning when
    memory is not enough for encoding a statement to UTF-8
    in Connection.call() and Cursor.execute().
  • UnicodEncodeError is now raised instead of sqlite3.Warning when
    the statement contains surrogate characters
    in Connection.__call__() and Cursor.execute().
  • TypeError is now raised instead of ValueError for non-string
    script argument in Cursor.executescript().
  • ValueError is now raised for script containing the null
    character instead of truncating it in Cursor.executescript().
  • Correctly handle exceptions raised when getting boolean value
    of the result of the progress handler.
  • Add may tests covering different exceptional cases.

https://bugs.python.org/issue44859

* MemoryError is now raised instead of sqlite3.Warning when memory is not enough for encoding a statement to UTF-8 in Connection.__call__() and Cursor.execute(). * UnicodEncodeError is now raised instead of sqlite3.Warning when the statement contains surrogate characters in Connection.__call__() and Cursor.execute(). * TypeError is now raised instead of ValueError for non-string script argument in Cursor.executescript(). * ValueError is now raised for script containing the null character instead of truncating it in Cursor.executescript(). * Correctly handle exceptions raised when getting boolean value of the result of the progress handler. * Add may tests covering different exceptional cases.

erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

LGTM, in general. Region coverage for util.c is now above 80%; sqlite3 coverage is getting better and better.

I left some minor comments.

Comment thread Misc/NEWS.d/next/Library/2021-08-07-17-28-56.bpo-44859.CCopjk.rst Outdated Show resolved Hide resolved
Comment thread Misc/NEWS.d/next/Library/2021-08-07-17-28-56.bpo-44859.CCopjk.rst Outdated Show resolved Hide resolved
Comment thread Modules/_sqlite/connection.c Outdated Show resolved Hide resolved
Comment thread Modules/_sqlite/cursor.c
}
} else {
PyErr_SetString(PyExc_ValueError, "script argument must be unicode.");
sql_len = strlen(sql_script);

erlend-aasland Aug 7, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

What about adding a custom AC converter that can extract the string length, so we don't have to iterate over the string again? Is it worth it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I think it is not worth. The code is simpler now (some checks were delegated to Argument Clinic), and it I prefer simplicity at the cost of additional strlen(). Custom converter would make it more complex than the original code. strlen() should be called in any case to check for embedded null characters. Now it is called 2 times instead of 1 time. The overhead is not too large.

Comment thread Modules/_sqlite/cursor.c
sql_len = strlen(sql_script);
int max_length = sqlite3_limit(self->connection->db,
SQLITE_LIMIT_LENGTH, -1);
if (sql_len >= (unsigned)max_length) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment
Suggested change
if (sql_len >= (unsigned)max_length) {
if (sql_len >= (size_t)max_length) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

How does it convert: int -> unsigned -> size_t or int -> ssize_t -> size_t? The standard specifies what the intermediate conversion the compiler should used, but without looking into it I cannot say that it will be the correct one.

If convert explicitly to unsigned, the conversion sequence will be unambiguous: int -> unsigned -> size_t.

Copy link
Copy Markdown
Contributor

BTW, that's a neat enhancement of the with_traceback decorator!

Copy link
Copy Markdown
Contributor

We should consider changing some more errors, as discussed in #27642 (comment), #27645 (comment), and #27645 (comment)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
serhiy-storchaka merged commit 0eec627 into python:main Aug 8, 2021
serhiy-storchaka deleted the sqlite-errors branch August 8, 2021 05:49
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Aug 12, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Footer

© 2026 GitHub, Inc.