Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3)

#9303: Migrate sqlite3 module to _v2 API to enhance performance

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 9 months ago by michi.schwarz
Modified:
3 years, 8 months ago
Reviewers:
jimjjewett
CC:
gh_ghaering.de, AntoinePitrou, haypo, larry, Arfrever, michi.schwarz_gmail.com, Petri Lehtinen, berkerpeksag, dimaqq, Jim.J.Jewett, storchaka, Robin.Schreiber, palaviv
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 1

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
configure.ac View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
Modules/_sqlite/connection.c View 1 2 6 chunks +10 lines, -5 lines 0 comments Download
Modules/_sqlite/cursor.c View 1 2 1 chunk +1 line, -1 line 0 comments Download
Modules/_sqlite/statement.c View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
Modules/_sqlite/util.h View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
pyconfig.h.in View 1 2 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 1
Jim.J.Jewett
7 years, 9 months ago #1
If the v2 calls are really exactly the same (except for the "_v2" in the name),
it might make sense to change them to a macro instead... My preference would be
that the macro turns into sqlite3_prepare_v2 if it exists, and sqlite3_prepare
if sqlite3_prepare_v2 does not exist, even a 

#define SQLITE_PREPARE sqlite_preare_v2

lets users fix the problem more easily if they are using an older system sqlite.
 (How old is older?)

http://bugs.python.org/review/9303/diff/4644/16497
File Modules/_sqlite/cursor.c (right):

http://bugs.python.org/review/9303/diff/4644/16497#newcode634
Modules/_sqlite/cursor.c:634: if (PyErr_Occurred()) {
All that deleted code doesn't look like a mechanical translation.  Could you
include some comments, or at least fodder for a commit message?  Why does it no
longer matter what the return code was?  Why are schema changes no longer
treated specially?
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+