search

sgilbertson / sqlite4java

Automatically exported from code.google.com/p/sqlite4java
0 stars 0 forks source link

Wrong column count for select without row #13

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The column count is set to 0 when a select statement returns no row. This is 
not the good value. Here is a use case (with empty database):
  - create table t (c text);
    * step returns false
    * columnCount returns 0
  - select * from sqlite_master
    * step returns true (one row for table t)
    * columnCount returns 5
  - select * from sqlite_master where name='not_exists'
    * step returns false
    * columnCount returns 0 but 5 was expected

This bug comes from two places:
  - how SQLiteStatement.step() handle SQLITE_DONE
  - the use a sqlite3_data_count

The step method sets myColumnCount to COLUMN_COUNT_UNKOWN when sqlite returns 
SQLITE_ROW and 0 when it returns SQLITE_DONE. But a select without row return 
SQLITE_DONE. So, line 322 of SQLiteStatement must be changed to:

myColumnCount = COLUMN_COUNT_UNKNOWN

(and maybe some code refractoring can be done).

The SQLiteStatement.ensureCorrectColumnCount(...) method use 
sqlite3_data_count. It returns the number of data in the result. If no result 
(no row) is returned, it returns 0. According to the following comment is 
sqlite3.c, sqlite3_column_count will returns the right column count:

** Set the number of result columns that will be returned by this SQL
** statement. This is now set at compile time, rather than during
** execution of the vdbe program so that sqlite3_column_count() can
** be called on an SQL statement before sqlite3_step().

So, line 1228 must be changed to:

myColumnCount = _SQLiteSwigged.sqlite3_column_count(handle);

I called sqlite3_column_count using reflection and obtained the right value.

A unit test is attached to this issue.

Original issue reported on code.google.com by olivier....@free.fr on 10 Aug 2010 at 8:35

Attachments:

GoogleCodeExporter commented 9 years ago 
Thanks for an extensive bug report. Will fix soon.

-- Igor

Original comment by ser...@gmail.com on 10 Aug 2010 at 9:09

GoogleCodeExporter commented 9 years ago 
I've attached a diff of my changes. But I'm not sure to handle all cases.

Original comment by olivier....@free.fr on 16 Aug 2010 at 10:16

Attachments:

GoogleCodeExporter commented 9 years ago 
There's another method to change in SQLiteStatement: the checkColumn method. 
Now, it's possible to retrieve column information when we don't have a row. So 
the following code, line 1223, must be removed:
    if (!myHasRow)
      throw new SQLiteException(WRAPPER_NO_ROW, null);

But as a side effect, columnBlob/Double/Int/... must include this code. I've 
created a checkHasRow() method.

New patch attached.

Original comment by olivier....@free.fr on 16 Aug 2010 at 1:09

Attachments:

GoogleCodeExporter commented 9 years ago 
It appears that sqlite3_data_count is safer than sqlite3_column_count, because 
it always returns the correct value or 0. In some cases sqlite3_column_count 
may return incorrect value (an example of that later). 

However, for the sake of convenience, I've changed to sqlite3_column_count and 
documenting the edge cases.

Original comment by ser...@gmail.com on 21 Aug 2010 at 10:15

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
I've fixed the column count, applied your patch (thanks) and did some 
refactoring. See in revision >= 192.

The case where sqlite3_column_count would return incorrect result is 
demonstrated in RegressionIssue13Tests.testUnstableColumnResult - 
http://code.google.com/p/sqlite4java/source/browse/trunk/test/com/almworks/sqlit
e4java/RegressionIssue13Tests.java#46

Will be delivered on Monday.

Original comment by ser...@gmail.com on 21 Aug 2010 at 10:39

GoogleCodeExporter commented 9 years ago 
Thanks.

I'm not sure the "testUnstableColumnResult" is really a bug. It's more like a 
design feature. Maybe you can add a comment columnCount about that.

Original comment by olivier....@free.fr on 23 Aug 2010 at 12:15

GoogleCodeExporter commented 9 years ago

Original comment by ser...@gmail.com on 23 Aug 2010 at 8:10