classification
Title: sqlite3: remove sqlite3_stmt_readonly()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: out of date
Dependencies: 28518 Superseder:
Assigned To: Nosy List: Ma Lin, berker.peksag, palaviv, palm.kevin, willingc
Priority: normal Keywords:

Created on 2017-01-24 04:38 by Ma Lin, last changed 2017-02-26 16:11 by berker.peksag. This issue is now closed.

Messages (7)
msg286130 - (view) Author: Ma Lin (Ma Lin) * Date: 2017-01-24 04:38
In CPython 3.6.0, sqlite3 module uses sqlite3_stmt_readonly(), however this function is only available on SQLite 3.7.4+ (release on 2010-12-07).
Then CPython 3.6.0 can not be compiled on RHEL6 (with SQLite 3.6.x), complaints: issue29098, issue29325.

sqlite3_stmt_readonly() was introduced in 284676cf2ac8, it was used twice [1][2], but it seems that we can avoid using this function in both of them.

[1] https://hg.python.org/cpython/file/3.6/Modules/_sqlite/cursor.c#l517
With palaviv's patches of issue28518, we can eliminate sqlite3_stmt_readonly() in here.

[2] https://hg.python.org/cpython/file/3.6/Modules/_sqlite/cursor.c#l612
We can use the old way in here, the old way is even better IMO.

This issue depends on issue28518.
msg288248 - (view) Author: Carol Willing (willingc) * Date: 2017-02-20 22:26
Another report of sqlite3 not compiling correctly on RHEL6 (https://github.com/jupyterhub/jupyterhub/issues/991). Is there currently a solution to build Python 3.6 with sqlite3 support?
msg288258 - (view) Author: Ma Lin (Ma Lin) * Date: 2017-02-21 03:25
> Is there currently a solution to build Python 3.6 with sqlite3 support?

I installed SQLite 3.16.2 on a Debian-based system with this way:
http://stackoverflow.com/questions/26261080
No idea whether it will work on RHEL 6.
msg288387 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-02-22 22:53
Thanks for the report, Ma.

> We can use the old way in here, the old way is even better IMO.

What do you mean by the old way? The use of switch statement?
msg288397 - (view) Author: Ma Lin (Ma Lin) * Date: 2017-02-23 02:33
The "old way" is not using sqlite3_stmt_readonly().

Before 3.6.0, we only count rowcount for INSERT, UPDATE, DELETE, REPLACE:
https://hg.python.org/cpython/file/3.5/Modules/_sqlite/cursor.c#l689

These four statements can be representd by is_dml in PR 245:
https://github.com/python/cpython/pull/245/commits/cbeabf4044e9e1563d228e359b0e7952f369b42a#diff-5a6abb9997d51e693f3b24986659c857R88

So only change this line is ok, then restore the behavior before 3.6.0 exactly:

- if (!sqlite3_stmt_readonly(self->statement->st)) {
+ if (self->statement->is_dml) {
    self->rowcount += (long)sqlite3_changes(self->connection->db);
} else {
    self->rowcount= -1L;
}

Why it's better?
sqlite3_changes() only works for INSERT, UPDATE, DELETE. see:
https://sqlite.org/c3ref/changes.html
So using sqlite3_stmt_readonly() to determine statements is not necessary at all.

In addition, we can add a comment for code safe in statement.c.
+    /* is_dml is used in two sites:
+        1, determine statements for implicit BEGIN.
+        2, determine statements for counting rowcount */
self->is_dml = (PyOS_strnicmp(p, "insert ", 7) == 0)
            || (PyOS_strnicmp(p, "update ", 7) == 0)
            || (PyOS_strnicmp(p, "delete ", 7) == 0)
            || (PyOS_strnicmp(p, "replace ", 8) == 0);
msg288409 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-02-23 05:53
> - if (!sqlite3_stmt_readonly(self->statement->st)) {
> + if (self->statement->is_dml) {

Ah, yes now I understand what do you mean! :) I agree and this is already in my local branch to fix issue 28518.
msg288600 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-02-26 16:11
The patch for issue 28518 has been merged and it doesn't contain any use of sqlite3_stmt_readonly(). Closing this as 'out of date'. Thanks again Ma!
History
Date User Action Args
2017-02-26 16:11:55berker.peksagsetstatus: open -> closed
resolution: out of date
messages: + msg288600

stage: resolved
2017-02-23 05:53:56berker.peksagsetmessages: + msg288409
2017-02-23 02:33:59Ma Linsetmessages: + msg288397
2017-02-22 22:56:53berker.peksaglinkissue29098 dependencies
2017-02-22 22:53:32berker.peksagsetdependencies: + execute("begin immediate") throwing OperationalError
messages: + msg288387
2017-02-21 03:25:47Ma Linsetmessages: + msg288258
2017-02-20 22:26:19willingcsetnosy: + willingc
messages: + msg288248
2017-02-15 09:07:42palm.kevinsetnosy: + palm.kevin
2017-01-24 04:38:13Ma Lincreate