System.Data.SQLite

Login
This project makes use of Eagle, provided by Mistachkin Systems.
Eagle: Secure Software Automation
2013-02-01
22:50 Closed ticket [c010fa6584]: Sign extension of unsigned 32-bit numbers. plus 3 other changes artifact: decfbb71ff user: mistachkin
22:49
Merge BindUInt32AsInt64 connection flag support to trunk. Pursuant to ticket [c010fa6584]. check-in: a0ead64064 user: mistachkin tags: trunk
21:00 Ticket [c010fa6584] Sign extension of unsigned 32-bit numbers. status still Fixed with 4 other changes artifact: 7e6067c9bd user: anonymous
2013-01-31
23:23 Ticket [c010fa6584]: 3 changes artifact: 357322ec5c user: mistachkin
23:19
Add BindUInt32AsInt64 connection flag to force binding of UInt32 values as Int64 instead. Pursuant to ticket [c010fa6584]. Closed-Leaf check-in: 99c7362ca8 user: mistachkin tags: tkt-c010fa6584
22:54 Ticket [c010fa6584] Sign extension of unsigned 32-bit numbers. status still Fixed with 3 other changes artifact: cf991ac5db user: anonymous
22:46 Ticket [c010fa6584]: 3 changes artifact: 873a830af1 user: mistachkin
22:00 Ticket [c010fa6584]: 3 changes artifact: 90b84a1ce6 user: anonymous
21:46 Ticket [c010fa6584]: 3 changes artifact: 24a8581bf3 user: anonymous
20:56 Ticket [c010fa6584]: 3 changes artifact: ac0ce98262 user: mistachkin
17:43 Ticket [c010fa6584]: 3 changes artifact: ba51d4ee1f user: anonymous
17:42 Ticket [c010fa6584]: 4 changes artifact: 3bf5e9ecf1 user: anonymous
07:48 Ticket [c010fa6584]: 4 changes artifact: ef71f658ff user: mistachkin
07:43 Fixed ticket [c010fa6584]. artifact: 47579f484c user: mistachkin
07:39
Merge fix for ticket [c010fa6584] to trunk. check-in: c55ee9c616 user: mistachkin tags: trunk
07:36
Avoid throwing overflow exceptions from the SQLite3.GetValue method for integral column types. Fix for [c010fa6584]. Closed-Leaf check-in: 618f5890e7 user: mistachkin tags: tkt-c010fa6584
02:25
Add test for ticket [c010fa6584]. check-in: 064c0bbe3d user: mistachkin tags: tkt-c010fa6584
2013-01-30
16:08 Ticket [c010fa6584] Sign extension of unsigned 32-bit numbers. status still Open with 3 other changes artifact: 76ffbd1135 user: anonymous
16:02 Ticket [c010fa6584]: 3 changes artifact: 17c31783d7 user: anonymous
04:06 Ticket [c010fa6584]: 3 changes artifact: 82319949a3 user: anonymous
04:02 Add attachment SQLiteSignBug.zip to ticket [c010fa6584] artifact: 755aec492e user: anonymous
02:34 Ticket [c010fa6584] Sign extension of unsigned 32-bit numbers. status still Open with 3 other changes artifact: fba1b385cc user: mistachkin
01:32 Ticket [c010fa6584]: 3 changes artifact: cbecc46a3d user: anonymous
01:02 Ticket [c010fa6584]: 3 changes artifact: 522172f15b user: mistachkin
00:48 Ticket [c010fa6584]: 3 changes artifact: 5e07f2ae39 user: mistachkin
00:19 Ticket [c010fa6584]: 6 changes artifact: 7c3f7942c8 user: mistachkin
2013-01-29
23:40 Ticket [c010fa6584]: 3 changes artifact: e95e91e058 user: anonymous
23:30 Ticket [c010fa6584]: 6 changes artifact: ef9c546c15 user: anonymous
23:23 New ticket [c010fa6584]. artifact: 9b720ffc39 user: anonymous

Ticket Hash: c010fa65845b29375501747e2611aef938659767
Title: Sign extension of unsigned 32-bit numbers.
Status: Closed Type: Code_Defect
Severity: Important Priority: NextRelease
Subsystem: Db_Type_Conversion Resolution: Fixed
Last Modified: 2013-02-01 22:50:18
Version Found In: 1.0.84.0
User Comments:
anonymous added on 2013-01-29 23:23:30:
The following code, which attempts to store UInt32.MaxValue into an existing row, actually stores -1L.

  using( var cmd = Connection.CreateCommand() )
  {
    cmd.CommandText = @" UPDATE SomeTable SET IntegerValue = @p2 WHERE ID = 1";
    Parameters.AddWithValue( "@p2", 4294967295 );
    var param = cmd.CreateParameter();
    param.ParameterName = "@p2";
    param.DbType = DbType.UInt32;
    param.Value = UInt32.MaxValue;
  }

Then, if you try to read it back like this, you get a conversion exception:

  cmd.CommandText = @" SELECT IntegerValue FROM SomeTable WHERE ID=1";
  using( var reader = cmd.ExecuteReader() )
    while( reader.Read() )
      Convert.ToUInt32( reader[0] );

Those two examples are more-or-less what the NHibernate ORM does when you declare a UInt32 property, set it to a value, and try to read it back.  Any values with the sign bit set will cause this problem.

The root of the problem is that UnsafeNativeMethods.sqlite3_bind_uint is aliased to:

  SQLITE_API int sqlite3_bind_int(sqlite3_stmt*, int, int);

That function call converts the value back to a int32_t, and then when sqlite's internals convert it to an int64_t, it's sign-extend.
  
Changing SQLite3.Bind_UInt32 to call UnsafeNativeMethods.sqlite3_bind_int64 instead of sqlite3_bind_uint solves the problem.

anonymous added on 2013-01-29 23:30:35:
By the way, strike this line.  Not sure how that got in there.
Parameters.AddWithValue( "@p2", 4294967295 );

anonymous added on 2013-01-29 23:40:45:
  using( var cmd = Connection.CreateCommand() )
  {
    cmd.CommandText = @" UPDATE SomeTable SET IntegerValue = @p2 WHERE ID = 1";
    var param = cmd.CreateParameter();
    param.ParameterName = "@p2";
    param.DbType = DbType.UInt32;
    param.Value = UInt32.MaxValue;
    cmd.Parameters.Add( param );
    cmd.ExecuteNonQuery();
  }

mistachkin added on 2013-01-30 00:19:43:
Can you provide the schema for the database table involved?  Also, I'm curious
about where the value is being converted to an Int64 by SQLite (or more likely
by System.Data.SQLite).

mistachkin added on 2013-01-30 00:48:31:
The SQLite core does not enforce types; however, System.Data.SQLite will use the
declared columns types in the table schema to convert the column values.  For
example, if a column is declared to be "INT", the Int32 CLR type will be used;
however, if a column is declared to be "INTEGER", the Int64 CLR type will be
used.  Having "INT" and "INTEGER" refer to two different underlying CLR types is
less than optimal; however, it cannot be changed now due to backward
compatibility.

mistachkin added on 2013-01-30 01:02:16:
Related check-in [ad2b42f3cc].  This check-in does not directly address the issue
in this ticket; however, it introduces ways to more explicitly declare a column
to be of a specific CLR type.

anonymous added on 2013-01-30 01:32:12:
I'll attach a working demo in the morning.  Columns are declared with INTEGER affinity.  I didn't dig down into the sqlite internals, so I can't say for sure what it's actually storing.  What comes out the other end is 0xffffffffffffffff when I was expecting 0xffffffff, however.

mistachkin added on 2013-01-30 02:34:19:
This issue is most likely not a bug.  The workaround would be to declare the
columns to be of type "INT".  Alternatively, if you can use the latest code on
trunk, you can declare the columns to be of type "UINT32".

anonymous added on 2013-01-30 04:06:51:
Test attached.  UInt16.MaxValue and byte.MaxValue (stored as DBType.UInt16 and DBType.UInt8, respectively) don't have the sign-extension problems, but they must call sqlite3_bind_int.  In that 8, 16, and 32-bit parameters don't handle sign-bit extensions the same, this is at least an inconsistency.

Unfortunately, the INT work-around doesn't work because Convert.ToUInt32( (int)-1 ) doesn't work.  Close, though.  I'll pull the trunk tomorrow and give UINT32 a shot.  I take it that the interpretation of INT/UINT32 is purely an System.Data.Sqlite thing?

anonymous added on 2013-01-30 16:02:52:
Trunk fails with a declared column type of UINT32 (test C3b in the attached test).  You've got something pretty similar to nhibernate's conversion call in your code base, at line 1922 in SQLite3.cs:

  return Convert.ChangeType(GetInt64(stmt, index), t, null);

And it fails if -1 is stored in the column because of this bug.

anonymous added on 2013-01-30 16:08:07:
I should add that commit [ad2b42f3cc] actually makes this bug worse, because now just accessing reader[i] on a UINT32 column in which you've stored a value > 2^31 throws an exception, where before you just got back a long containing an difficult-to-convert number.

mistachkin added on 2013-01-31 07:43:26:
This issue should now be fixed on trunk by check-in [c55ee9c616].

In your example as it was originally uploaded there are still several things
that do not work; however, the parameter and column handling for unsigned types
is now correct.

To use the UInt32 CLR type, the database column must be declared to be one of
the following:

    UINT
    UINT32
    UNSIGNEDINTEGER
    UNSIGNEDINTEGER32

Using other database column types, most notably INT and INTEGER, will not
produce the correct results in all cases.

Please let me know if the latest trunk changes clear the issue for you.

anonymous added on 2013-01-31 17:42:38:
That works, but only if all your applications use system.data.sqlite.  Consider how it looks to an another app calling sqlite3_column_int64() directly.  I'd expect to be able to load a uint32_t from my database into a int64_t without extra casting/masking. Example attached.

I'm not sure if I should re-open this or not, so sorry if I've annoyed you.  You've been very helpful so far, and I appreciate it.

anonymous added on 2013-01-31 17:43:47:
Hmm, it's not letting me attach files.  I'll try again this evening.

mistachkin added on 2013-01-31 20:56:23:
Internally, the SQLite core library more-or-less stores integers as 64-bits and
then returns only the bottom 32-bits when the sqlite3_column_int() function is
called.  Also, it does not directly support unsigned integer types.

Therefore, storing 0xFFFFFFFF into a column with INTEGER type affinity will really
result in "-1" being stored.  Subsequently, if the sqlite3_column_int() function
is used to fetch that value, it will be correct.  However, using the
sqlite3_column_int64() function will also return "-1", which will not result in
the correct unsigned value if a cast is applied.

After the referenced fix to System.Data.SQLite, it should be able to avoid the
above situation by using the correct column value function and properly casting
the returned value to the appropriate data type.

anonymous added on 2013-01-31 21:46:29:
I guess it's just a matter of opinion at this point.  If I'm storing uint32_t values in sqlite, I'd use sqlite3_column_int64 because then you get the logical value into the database and don't have to explain to someone else using it that all those negative values need to be cast back to uint32_t values before they can be used.  But since you're stuck doing that anyway for uint64_t... eh, whatever.

anonymous added on 2013-01-31 22:00:58:
And, it occurs to me, storing uint32_t values with sqlite3_column_int rather than sqlite3_column_int64 will mean that you can't do > or < comparisons or sorting correctly via SQL.  0x80000000 should be > than 0x01, but won't be.  Does sqlite let you do c-style casts in queries?

mistachkin added on 2013-01-31 22:46:38:
The SQLite core library does not directly support C-style casts to integer types
with a specific width.  Similarly, it does not directly support the concept of
unsigned integer types.

anonymous added on 2013-01-31 22:54:19:
But it does support a signed 64-bit insert : )  Ah well, looks like I'll just have to declare everything as Int64 in nhibernate and add a bunch of casts where my code needs UInt32s.

mistachkin added on 2013-01-31 23:23:03:
I've just checked-in a workaround for the UInt32 issue via a new
BindUInt32AsInt64 connection flag to force binding of all UInt32
values as Int64 instead.

Will this work for your use cases?

You would need to add the following in all your connection strings:

    Flags=BindUInt32AsInt64;

The new flag code is currently on the 'tkt-c010fa6584' branch.

anonymous added on 2013-02-01 21:00:29:
Works great so far.  I re-declared my columns as UINT32, changed my nhibernate object properties to UInt32s, and set BindUInt32AsInt64.  Now NHibernate doesn't throw exceptions, I don't have to caste anything, and SQL order-by commands work correctly.  

Thanks for all your work on this.  Is this going to be making it into your March release?

mistachkin added on 2013-02-01 22:50:18:
Fixed on trunk by check-in [a0ead64064].

Yes, this will make the March release.  Thanks for your feedback.

Attachments: