System.Data.SQLite
View Ticket
Not logged in
Ticket UUID: 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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
Hmm, it's not letting me attach files.  I'll try again this evening.

mistachkin added on 2013-01-31 20:56:23: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
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: (text/x-fossil-plain)
Fixed on trunk by check-in [a0ead64064].

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

Attachments: