classification
Title: sqlite: broken long integer handling for arguments to user-defined functions
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BinaryMan32, amaury.forgeotdarc, brian.curtin, flupke, ghaering, loewis, petri.lehtinen, python-dev, santoso.wijaya
Priority: normal Keywords: patch

Created on 2010-03-01 02:03 by BinaryMan32, last changed 2012-02-21 19:04 by petri.lehtinen. This issue is now closed.

Files
File name Uploaded Description Edit
sqlite_long_test.py BinaryMan32, 2010-03-01 02:03 test program demonstrating failure
broken_long_int_sqlite_functions.diff flupke, 2010-11-05 23:06 review
broken_long_sqlite_userfunctions.diff flupke, 2011-01-08 16:22 review
Messages (11)
msg100235 - (view) Author: Fred Fettinger (BinaryMan32) Date: 2010-03-01 02:03
Handling of long integers is broken for arguments to sqlite functions created with the create_function api. Integers passed to a sqlite function are always converted to int instead of long, which produces an incorrect value for integers outside the range of a int32 on a 32 bit machine. These failures cannot be reproduced on a 64 bit machine, since sizeof(long) == sizeof(long long).

I attached a program that demonstrates the failure. This program reports failures on a 32 bit machine, and all tests pass on a 64 bit machine.
msg100237 - (view) Author: Fred Fettinger (BinaryMan32) Date: 2010-03-01 02:22
I've never really looked at the python source before, but this is my best guess at the problem:

For the standard SELECT query:
In Modules/_sqlite/cursor.c, _pysqlite_fetch_one_row() has this code:
PY_LONG_LONG intval;
...
} else if (coltype == SQLITE_INTEGER) {
    intval = sqlite3_column_int64(self->statement->st, i);
    if (intval < INT32_MIN || intval > INT32_MAX) {
        converted = PyLong_FromLongLong(intval);
    } else {
        converted = PyInt_FromLong((long)intval);
    }
}

For user-defined function arguments:
In Modules/_sqlite/connection.c, _pysqlite_build_py_params() has this code:
PY_LONG_LONG val_int;
...
case SQLITE_INTEGER:
    val_int = sqlite3_value_int64(cur_value);
    cur_py_value = PyInt_FromLong((long)val_int);
    break;

A select query can return long integers from C to python but a user-defined function argument cannot.
msg120555 - (view) Author: Philippe Devalkeneer (flupke) Date: 2010-11-05 23:06
Hello,
Here is a patch to fix it :) 
( and don't blame me too much if something is not correct, it's the first patch I submit :) )

Philippe
msg120801 - (view) Author: Philippe Devalkeneer (flupke) Date: 2010-11-08 21:02
The regression tests do not work anymore in test_sqlite with the patch, I will check more in details why... sorry.
msg120802 - (view) Author: Philippe Devalkeneer (flupke) Date: 2010-11-08 21:23
No, I made a mistake in my build, test_sqlite runs actually fine before and after the patch.
msg125421 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-05 13:03
- unit tests are needed.
- Py_LONG_LONG should be used instead of "long long", and the cast removed.
- The patch assumes that longlong == int64, but is it true on all platforms?
msg125455 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-05 19:12
Alternatively, val_int should have type sqlite3_int64, which is the return type of sqlite3_value_int64.
msg125535 - (view) Author: Philippe Devalkeneer (flupke) Date: 2011-01-06 09:30
Ok I will redo a patch in the next few days with possibly type sqlite3_int64, and include tests.

Thank you for the review.
msg125789 - (view) Author: Philippe Devalkeneer (flupke) Date: 2011-01-08 16:22
Here a new patch with sqlite3_int64 type, and unit test.
msg153869 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-21 12:11
New changeset e67715b87131 by Petri Lehtinen in branch '3.2':
sqlite3: Fix 64-bit integer handling in user functions on 32-bit architectures
http://hg.python.org/cpython/rev/e67715b87131

New changeset cf12fe9ce2b0 by Petri Lehtinen in branch 'default':
Merge branch '3.2'
http://hg.python.org/cpython/rev/cf12fe9ce2b0

New changeset 789a3ea97083 by Petri Lehtinen in branch '2.7':
sqlite3: Fix 64-bit integer handling in user functions on 32-bit architectures
http://hg.python.org/cpython/rev/789a3ea97083
msg153895 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-02-21 19:04
Thanks for the patches, Philippe! I committed the latest one after some modifications.
History
Date User Action Args
2012-02-21 19:04:01petri.lehtinensetmessages: + msg153895
2012-02-21 12:11:45python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg153869

resolution: fixed
stage: test needed -> resolved
2012-02-08 20:42:45petri.lehtinensetnosy: + petri.lehtinen
2011-01-08 16:22:40flupkesetfiles: + broken_long_sqlite_userfunctions.diff
nosy: loewis, ghaering, amaury.forgeotdarc, brian.curtin, BinaryMan32, santoso.wijaya, flupke
messages: + msg125789
2011-01-06 09:30:49flupkesetnosy: loewis, ghaering, amaury.forgeotdarc, brian.curtin, BinaryMan32, santoso.wijaya, flupke
messages: + msg125535
2011-01-05 19:12:51loewissetnosy: + loewis
messages: + msg125455
2011-01-05 13:03:22amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg125421
2011-01-05 10:29:50eric.araujosetnosy: ghaering, brian.curtin, BinaryMan32, santoso.wijaya, flupke
stage: needs patch -> test needed
components: + Extension Modules, - Library (Lib)
versions: + Python 2.7, Python 3.2, - Python 2.6
2010-11-08 21:23:03flupkesetmessages: + msg120802
2010-11-08 21:02:40flupkesetmessages: + msg120801
2010-11-06 00:40:21santoso.wijayasetnosy: + santoso.wijaya
2010-11-05 23:06:34flupkesetfiles: + broken_long_int_sqlite_functions.diff

nosy: + flupke
messages: + msg120555

keywords: + patch
2010-03-01 03:40:53brian.curtinsetstage: test needed -> needs patch
2010-03-01 03:40:35brian.curtinsetpriority: normal
nosy: + brian.curtin
stage: test needed

versions: + Python 3.1
2010-03-01 02:22:17BinaryMan32setmessages: + msg100237
2010-03-01 02:06:13BinaryMan32setnosy: + ghaering
2010-03-01 02:03:09BinaryMan32create