#13 Wrong column count for select without row

[Anonymous]

Originally created by: olivier....@free.fr

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.

[Anonymous]

Originally posted by: ser...@gmail.com

Thanks for an extensive bug report. Will fix soon.

-- Igor

Status: Accepted

[Anonymous]

Originally posted by: olivier....@free.fr

I've attached a diff of my changes. But I'm not sure to handle all cases.

[Anonymous]

Originally posted by: olivier....@free.fr

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.

[Anonymous]

Originally posted by: ser...@gmail.com

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.

[Anonymous]

Originally posted by: ser...@gmail.com

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 - https://code.google.com/p/sqlite4java/source/browse/trunk/test/com/almworks/sqlite4java/RegressionIssue13Tests.java#46

Will be delivered on Monday.

Status: Fixed

[Anonymous]

Originally posted by: olivier....@free.fr

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.

[Anonymous]

Originally posted by: ser...@gmail.com

(No comment was entered for this change.)

Labels: FixVersion-201
Status: Delivered