lunarmodules / luasql

LuaSQL is a simple interface from Lua to a DBMS.
http://lunarmodules.github.io/luasql
547 stars 192 forks source link

sqlite3 32-bit problem #43

Closed ghost closed 8 years ago

ghost commented 8 years ago

In a 32 bit build, retrieving int columns results in the number truncated to 32 bits.

Test case:

sqlite3 test.sqlite 'create table T(n integer);insert into T values(4294967297);'
echo 'print(require "luasql.sqlite3".sqlite3():connect "temp.sqlite":execute "select * from T":fetch())' > test.lua
lua test.lua

Prints 1 on systems where ptrdiff_t is 32 bits.

The following fix extends it to the maximum precision available with Lua (beyond that it's always possible to cast to string, which is the workaround I've used for now):

diff --git a/src/ls_sqlite3.c b/src/ls_sqlite3.c
index 150073c..b2acbe1 100644
--- a/src/ls_sqlite3.c
+++ b/src/ls_sqlite3.c
@@ -126,7 +126,7 @@ static int finalize(lua_State *L, cur_data *cur) {
 static void push_column(lua_State *L, sqlite3_stmt *vm, int column) {
   switch (sqlite3_column_type(vm, column)) {
   case SQLITE_INTEGER:
-    lua_pushinteger(L, sqlite3_column_int64(vm, column));
+    lua_pushnumber(L, (lua_Number)sqlite3_column_int64(vm, column));
     break;
   case SQLITE_FLOAT:
     lua_pushnumber(L, sqlite3_column_double(vm, column));
hishamhm commented 8 years ago

@pgimeno is that on Lua 5.1, 5.2 or 5.3?

I believe in Lua 5.3 this will return a 64-bit int correctly (even if the machine in 32-bit).

If my guess is correct, I believe this fix should have an #ifdef so that it retains full 64-bit precision in Lua 5.3, and gets 53-bit precision in Lua 5.2 (or maybe check the size of the number and return a string if greater than 2^53?).

ghost commented 8 years ago

That seems right. I didn't know about Lua 5.3's support for integers. I tested with 5.1.

The last consecutive integer that a double can store is 9007199254740992; past that, using a string seems sensible. It might even be possible to check if the value can be stored in a double (by casting to double then back to 64-bit integer and checking if they differ), and return it as a string if not; but that means that some string values would be interspersed with some number values, which is arguably a violation of the "principle of least astonishment".

hishamhm commented 8 years ago

Yeah, I just tested this, for fun:

diff --git a/src/ls_sqlite3.c b/src/ls_sqlite3.c
index 150073c..1a44f38 100644
--- a/src/ls_sqlite3.c
+++ b/src/ls_sqlite3.c
@@ -10,6 +10,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
+#include <math.h>

 #include "sqlite3.h"

@@ -126,7 +127,23 @@ static int finalize(lua_State *L, cur_data *cur) {
 static void push_column(lua_State *L, sqlite3_stmt *vm, int column) {
   switch (sqlite3_column_type(vm, column)) {
   case SQLITE_INTEGER:
+#if LUA_VERSION_NUM >= 503
     lua_pushinteger(L, sqlite3_column_int64(vm, column));
+#else
+    {
+      sqlite3_int64 value = sqlite3_column_int64(vm, column);
+      if (value <= pow(2, 53) && value >= -(pow(2, 53) + 1))
+        {
+          lua_pushnumber(L, value);
+        }
+      else
+        {
+          char strvalue[50];
+          snprintf(strvalue, sizeof(strvalue)-1, "%lld", value);
+          lua_pushstring(L, strvalue);
+        }
+    }
+#endif
     break;
   case SQLITE_FLOAT:
     lua_pushnumber(L, sqlite3_column_double(vm, column));

It works and it preserves data, but yeah, it may be a bit too magic — and worse, it would behave differently from the other LuaSQL drivers for no very good reason. Perhaps we should use just this instead:

diff --git a/src/ls_sqlite3.c b/src/ls_sqlite3.c
index 150073c..c39dd9c 100644
--- a/src/ls_sqlite3.c
+++ b/src/ls_sqlite3.c
@@ -126,7 +126,12 @@ static int finalize(lua_State *L, cur_data *cur) {
 static void push_column(lua_State *L, sqlite3_stmt *vm, int column) {
   switch (sqlite3_column_type(vm, column)) {
   case SQLITE_INTEGER:
+#if LUA_VERSION_NUM >= 503
     lua_pushinteger(L, sqlite3_column_int64(vm, column));
+#else
+    /* Preserves precision of integers up to 2^53. */
+    lua_pushnumber(L, sqlite3_column_int64(vm, column));
+#endif
     break;
   case SQLITE_FLOAT:
     lua_pushnumber(L, sqlite3_column_double(vm, column));

The behavior would be consistent with that of Lua 5.{1,2} numbers in general....

ghost commented 8 years ago

That works. Those who need to handle bigger numbers can use select cast(intcolumn as varchar) ... as I did.

hishamhm commented 8 years ago

Cool, I committed the second version. Closing this, thanks for reporting!

tomasguisasola commented 8 years ago

I'd like to point out that LuaSQL drivers implement different rules when internalizing values from the database to Lua. PostgreSQL driver, for instance, DOES NOT convert any value, except NULL (which is converted to nil). This is done according to PostgreSQL API which naturally provides strings when obtaining the results.

Regards, Tomás

On 2016-03-13 19:13, Hisham Muhammad wrote:

Yeah, I just tested this, for fun:

diff --git a/src/ls_sqlite3.c b/src/ls_sqlite3.c index 150073c..1a44f38 100644 --- a/src/ls_sqlite3.c +++ b/src/ls_sqlite3.c @@ -10,6 +10,7 @@

include

include

include

+#include

include "sqlite3.h"

@@ -126,7 +127,23 @@ static int finalize(lua_State L, cur_data cur) { static void push_column(lua_State L, sqlite3_stmt vm, int column) { switch (sqlite3_column_type(vm, column)) { case SQLITE_INTEGER: +#if LUA_VERSION_NUM >= 503 lua_pushinteger(L, sqlite3_column_int64(vm, column)); +#else

  • {
  • sqlite3_int64 value = sqlite3_column_int64(vm, column);
  • if (value <= pow(2, 53) && value >= -(pow(2, 53) + 1))
  • {
  • lua_pushnumber(L, value);
  • }
  • else
  • {
  • char strvalue[50];
  • snprintf(strvalue, sizeof(strvalue)-1, "%lld", value);
  • lua_pushstring(L, strvalue);
  • }
  • } +#endif break; case SQLITE_FLOAT: lua_pushnumber(L, sqlite3_column_double(vm, column));

It works and it preserves data, but yeah, it may be a bit too magic — and worse, it would behave differently from the other LuaSQL drivers for no very good reason. Perhaps we should use just this instead:

diff --git a/src/ls_sqlite3.c b/src/ls_sqlite3.c index 150073c..c39dd9c 100644 --- a/src/ls_sqlite3.c +++ b/src/ls_sqlite3.c @@ -126,7 +126,12 @@ static int finalize(lua_State L, cur_data cur) { static void push_column(lua_State L, sqlite3_stmt vm, int column) { switch (sqlite3_column_type(vm, column)) { case SQLITE_INTEGER: +#if LUA_VERSION_NUM >= 503 lua_pushinteger(L, sqlite3_column_int64(vm, column)); +#else

  • /* Preserves precision of integers up to 2^53. */
  • lua_pushnumber(L, sqlite3_column_int64(vm, column)); +#endif break; case SQLITE_FLOAT: lua_pushnumber(L, sqlite3_column_double(vm, column));

The behavior would be consistent with that of Lua 5.{1,2} numbers in general....

Reply to this email directly or view it on GitHub [1].

*

Links:

[1] https://github.com/keplerproject/luasql/issues/43#issuecomment-196063071