[Monotone-devel] Updated Issue 96 - SQLite 3.7.3 breakage (monotone)

[code]

Hello,

The following issue has been updated:

96 - SQLite 3.7.3 breakage
Project: monotone
Status: Fixed
Reported by: Thomas Keller
URL: http://code.monotone.ca/p/monotone/issues/96/
Labels:
 Type:Defect
 Priority:Critical

Comments (last first):

# By Thomas Keller, Oct 21, 2010:

Many thanks Tim for the fix! I got some clarification from Richard Hipp on the 
issue in the meantime:

> There were cases in 3.7.2 and earlier where 
> sqlite3_column_blob() would return a NULL pointer
> for a zero-length blob.  And this was documented.
> But the behavior was inconsistent, which seemed wrong.
> So we modified it to do the same thing every time.

Apparently we always go a zero-length blob back and thought this was the 
correct behaviour. Sadly, this change wasn't mentioned under 
http://sqlite.org/releaselog/3_7_3.html - maybe it will be in the future.

# By Timothy Brownawell, Oct 21, 2010:

As best I can tell, it only returns zero for empty blobs and NULLs. So the 
following should work (committed as 9def7716bcd068fc1929ed8336432bbed775d9ee).


#
# old_revision [97939c9677047b36beef031cce4c1896849a987c]
#
# patch "database.cc"
#  from [0afa3ff4bd9c9ee3bc62b10bcf6295a9f5388d64]
#    to [8bfff559a0894259fe3668294bd3906ae837129b]
#
============================================================
--- database.cc 0afa3ff4bd9c9ee3bc62b10bcf6295a9f5388d64
+++ database.cc 8bfff559a0894259fe3668294bd3906ae837129b
@@ -1531,12 +1531,19 @@ database_impl::fetch(results & res,
       vector<string> row;
       for (int col = 0; col < ncol; col++)
         {
+          // We never store NULLs, so we should never see one.
+          int const datatype = sqlite3_column_type(i->second.stmt(), col);
+          E(datatype != SQLITE_NULL, origin::database,
+            F("null result in query: %s") % query.sql_cmd);
           const char * value = (const 
char*)sqlite3_column_blob(i->second.stmt(), col);
           int bytes = sqlite3_column_bytes(i->second.stmt(), col);
-          E(value, origin::database,
-            F("null result in query: %s") % query.sql_cmd);
-          row.push_back(string(value, value + bytes));
-          //L(FL("row %d col %d value='%s'") % nrow % col % value);
+          if (value) {
+            row.push_back(string(value, value + bytes));
+          } else {
+            // sqlite3_column_blob() returns null for zero-length
+            I(bytes == 0);
+            row.push_back(string());
+          }
         }
       res.push_back(row);
     }

 Status: Fixed

# By Thomas Keller, Oct 18, 2010:

Ok, I debugged into this and the easiest way to reproduce the problem in 3.7.3 
is to do

$ mtn db init -d test.mtn
$ mtn db set foo foo "" -d test.mtn
$ mtn ls vars

The empty value column is returned as "" (zero-length string) in sqlite 3.7.2 
and 3.6.23 (and possibly other versions which used to work), but as 0 in 3.7.3.

According to sqlite's docs (http://www.sqlite.org/c3ref/column_blob.html) this 
seems to be correct

  The return value from sqlite3_column_blob() for a 
  zero-length BLOB is a NULL pointer."

but apparently was broken in earlier versions (or the paragraph has just been 
added for 3.7.3).

I'm now trying to find out whether there is a valid error case for which 
sqlite3_column_blob() also returns 0 and if not, our error in database.cc 
should probably laxed and the null pointer handled properly.

# By Thomas Keller, Oct 16, 2010:

Steps to reproduce the problem:
-------------------------------

$ mtn ls vars

Expected result:
----------------

Database variables

Actual results:
---------------

mtn: fatal: error: null result in query: SELECT domain, name, value FROM db_vars
mtn: this is almost certainly a bug in monotone.
...

Output of `mtn version --full`:
-------------------------------

monotone 0.99dev (base revision: 104f634f6a14e8e085ba5c89d8b95d16ae5b62c4)
Running on          : Darwin 9.8.0 Darwin Kernel Version 9.8.0: Wed Jul 15 
16:55:01 PDT 2009; root:xnu-1228.15.4~1/RELEASE_I386 i386
C++ compiler        : GNU C++ version 4.0.1 (Apple Inc. build 5490)
C++ standard library: GNU libstdc++ version 20050421
Boost version       : 1_44
SQLite version      : 3.7.3 (compiled against 3.7.2)
Lua version         : Lua 5.1
PCRE version        : 8.10 2010-06-25 (compiled against 8.10)
Botan version       : 1.8.10 (compiled against 1.8.10)
Changes since base revision:
format_version "1"

new_manifest [c679fc930e31daba26c09dc437f950a0d7fe7d78]

old_revision [104f634f6a14e8e085ba5c89d8b95d16ae5b62c4]


-----------

This seems to be a runtime-only problem, i.e. it doesn't matter that mtn is 
compiled against 3.7.2 for the problem to pop up. The invariant in 
database.cc:1537 fires, because a column we're trying to fetch returns a NULL 
result:

// value is NULL
const char * value = \
  (const char*)sqlite3_column_blob(i->second.stmt(), col);

I'm not sure why this is the case and I cannot take on this right now. I hope I 
can evaluate the problem more in detail later on.



--
Issue: http://code.monotone.ca/p/monotone/issues/96/