Repository navigation
bpo-44859: Improve error handling in sqlite3 and change some errors - #27654
serhiy-storchaka merged 2 commits into
Conversation
Sorry, something went wrong.
* 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
There was a problem hiding this 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.
Sorry, something went wrong.
| } | ||
| } else { | ||
| PyErr_SetString(PyExc_ValueError, "script argument must be unicode."); | ||
| sql_len = strlen(sql_script); |
There was a problem hiding this 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?
Sorry, something went wrong.
There was a problem hiding this 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.
Sorry, something went wrong.
| sql_len = strlen(sql_script); | ||
| int max_length = sqlite3_limit(self->connection->db, | ||
| SQLITE_LIMIT_LENGTH, -1); | ||
| if (sql_len >= (unsigned)max_length) { |
There was a problem hiding this comment.
| if (sql_len >= (unsigned)max_length) { | |
| if (sql_len >= (size_t)max_length) { |
Sorry, something went wrong.
There was a problem hiding this 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.
Sorry, something went wrong.
|
BTW, that's a neat enhancement of the with_traceback decorator! |
Sorry, something went wrong.
|
We should consider changing some more errors, as discussed in #27642 (comment), #27645 (comment), and #27645 (comment) |
Sorry, something went wrong.
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
memory is not enough for encoding a statement to UTF-8
in Connection.call() and Cursor.execute().
the statement contains surrogate characters
in Connection.__call__() and Cursor.execute().
script argument in Cursor.executescript().
character instead of truncating it in Cursor.executescript().
of the result of the progress handler.
https://bugs.python.org/issue44859