php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64531 SQLite3Result::fetchArray runs the query again.
Submitted: 2013-03-27 09:20 UTC Modified: 2018-09-24 15:49 UTC
Votes:49
Avg. Score:4.6 ± 0.7
Reproduced:47 of 47 (100.0%)
Same Version:18 (38.3%)
Same OS:8 (17.0%)
From: phplists at stanvassilev dot com Assigned:
Status: Verified Package: SQLite related
PHP Version: 5.4.13 OS: Windows XP SP3 32bit
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2013-03-27 09:20 UTC] phplists at stanvassilev dot com
Description:
------------
I was quite surprised to find that, but SQLite3 results cause the query to execute 
a second time when you try to fetch from them (after the query() call).

This is not just a harmless performance issue. When the query is an INSERT query, 
it causes duplicate rows, and creates all kinds of other messes.

IMPORTANT: If you perform the same tests with the SQLite3 driver for PDO, it 
doesn't have this issue (fetching results won't cause the query to run again). The 
issue is specific to the SQLite3 extension.

Test script:
---------------
EXAMPLE1:

I caught that when I run an INSERT query through a generic routine which always uses query() and then runs fetchArray() in a loop to see if something was returned. Naturally INSERT queries return nothing, but trying to fetch on an empty result set should NOT produce side effects:

$conn = new SQLite3('Log.sqlite', \SQLITE3_OPEN_READWRITE);

$res = $conn->query('INSERT INTO Table VALUES(null, 1, 2, 3'); // Inserts a row here (the null is an auto-incrementing PK).

$res->fetchArray()); // Inserts the *same* row data as above, again (with the next auto-incrementing PK number).
$res->fetchArray()); // And yet again...

EXAMPLE2: 

Another way to prove that something is fishy, is by registering a PHP function for use in SQLite3. Let's say we have a table with a column "id", and we have three rows, with "id" values 1, 2, 3.

function testing($val) {
	echo 'Testing with: ' . $val . '<br>';
	return true;
}

$conn = new SQLite3('Log.sqlite', \SQLITE3_OPEN_READWRITE);

$conn->createFunction('testing', 'testing', 1);

$res = $conn->query('SELECT * FROM Table WHERE testing(id)'); // "Testing with: 1"

$arr = $res->fetchArray(); // "Testing with: 1" (notice the repetition of 1 with the query above, this shouldn't occur).
$arr = $res->fetchArray(); // "Testing with: 2"
$arr = $res->fetchArray(); // "Testing with: 3"

// At this point the next call to fetchArray() will return false, as it should. But what's happening internally? Something else:

$arr = $res->fetchArray(); // "Testing with: 1" again. Huh? Why is it running that again?
$arr = $res->fetchArray(); // "Testing with: 2" 
$arr = $res->fetchArray(); // "Testing with: 3"
$arr = $res->fetchArray(); // "Testing with: 1"
$arr = $res->fetchArray(); // "Testing with: 2"
$arr = $res->fetchArray(); // "Testing with: 3"
// ...and so on forever.

Another used has encountered an issue like this over 6 months ago (!) which means this bug has been around for a while: http://stackoverflow.com/questions/12981332/php-sqlite3resultfetcharray-re-executes-query

Expected result:
----------------
The SQLite3 extension should execute a query once, and each row of the result set 
should be executed once, like PDO's SQLite3 driver does.

Fetching should cause duplicate INSERTS and twice computed results.

Actual result:
--------------
The SQLite3 extension executes the query again when fetching from the result set.

Patches

fix64531-sqlite3-fetchArray-skipNoColumns (last revision 2020-02-01 00:13 UTC by antoni at friki dot cat)

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-03-27 09:25 UTC] phplists at stanvassilev dot com
I hate when that happens, although I guess I'm clear:

Typo in Expected Result: "Fetching should cause duplicate";
should be:"Fetching should NOT cause duplicate";

Typo in EXAMPLE2: "Another used has encountered an issue like"
should be: "Another user has encountered an issue like"
 [2013-03-27 16:44 UTC] frozenfire@php.net
The related source is: https://github.com/php/php-
src/blob/master/ext/sqlite3/sqlite3.c#L1725

The solution to this bug might simply have to be a documentation note indicating 
the SQLite3::exec should be used for inserts instead of SQLite3::query, as 
::query will necessarily re-execute to "step" through.
 [2013-03-27 16:59 UTC] phplists at stanvassilev dot com
Before people start recommending a documentation "fix", be aware that I have two 
examples.

The first use case is solved by using exec(). The second is solved by nothing, 
the query is evaluated twice. Those are just two examples demonstrating the same 
issue.

Don't try to fix my examples, try to fix the issue. The culprit seems to be that 
_step is executed for the query once on query(), but then it's executed all over 
again on first fetch, starting at row1 again.

A proposed solution fixing all side effects would be to run step() on query() 
and cache the fetched result, then return it on first fetch, then call step() on 
second fetch etc.:

query(...); // call sqlite3_step, save row1
fetchArray(...); // return saved row1
fetchArray(...); // call sqlite3_step, return row2
fetchArray(...); // call sqlite3_step, return row3
fetchArray(...); // call sqlite3_step, return row4
...
 [2013-03-27 17:23 UTC] phplists at stanvassilev dot com
I've been discussing this in #PHP.PECL, with auroraeos and others and basically 
the problem is caused by the OOP interface in the binding.

The binding introduces extra logic so query() in PHP can return a resultset 
object or false etc.

In order to know what to return, query() calls sqlite3_step() which fetches the 
first row of the result.

So far so good. Here's the problem:

The binding then *resets* the query so the first call to fetchArray() returns 
row1 again. This is not library behavior, it's binding behavior that's 
additional logic.

This causes row1 to be computed twice, and queries to run twice etc.

The solution for solving this without introducing BC breaks and interface 
breaks, is for the binding to store the row it fetched during query() with the 
resultset, as outlined in my previous comment, and return it on first call to 
fetchArray, without calling step.

On subsequent calls to fetchArray(), step is called to fetch the other rows.

Either way you look at it, the binding added behavior that isn't in the original 
library, and this is causing performance issues and side effects. The 
responsible thing is to keep the PHP OOP interface compatible, but fix the 
performance issues and side effects.

And what I described is how to do it...
 [2014-12-27 11:21 UTC] yuri dot kanivetsky at gmail dot com
I've run into it trying to do:

    function sq($query, $params = array()) {
        if ($params) {
            $stmt = sdb()->prepare($query);
            if ( ! $stmt) {
                die("sqlite: " . sdb()->lastErrorMsg());
            }
            foreach ($params as $param) {
                $r = call_user_func_array(array($stmt, 'bindValue'), $param);
                if ( ! $r) {
                    die("sqlite: " . sdb()->lastErrorMsg());
                }
            }
            $res = $stmt->execute();
            if ( ! $res) {
                die("sqlite: " . sdb()->lastErrorMsg());
            }
        } else {
            $res = sdb()->query($query);
            if ( ! $res) {
                die("sqlite: " . sdb()->lastErrorMsg());
            }
        }
        $r = array();
        while ($row = $res->fetchArray(SQLITE3_ASSOC)) {
            $r[] = $row;
        }
        return $r;
    }

Made it work by wrapping the last part in `if ($res->numColumns()) {`:

        ...
        }
        if ($res->numColumns()) {
            $r = array();
            while ($row = $res->fetchArray(SQLITE3_ASSOC)) {
                $r[] = $row;
            }
            return $r;
        }
    }
 [2015-07-18 13:49 UTC] dupa at dupa dot com
Same problem here, duplicate inserts with "query" method.
 [2016-06-27 15:43 UTC] cmb@php.net
-Status: Open +Status: Verified
 [2016-06-27 15:43 UTC] cmb@php.net
Confirmed: <https://3v4l.org/MMGio>. And yes, this is a bug, and
not a documentation problem (nonetheless, DDL and DML statements
should use ::exec()).
 [2016-06-27 18:07 UTC] cmb@php.net
-Summary: SQLite3's SQLite3Result::fetchArray runs the query again. +Summary: SQLite3Result::fetchArray runs the query again.
 [2018-01-31 16:37 UTC] thomas dot loch at fusion-core dot net
I've just encountered this issue on Linux and Version 5.6.30 while using a similar generic wrapper around prepare/bind*/execute(). Is there still intent to fix this after five years?
 [2018-09-24 15:49 UTC] cmb@php.net
While the fix for SQLite3::query() is trivial, a general fix for
SQLite3Stmt::execute() is impossible, since it is allowed to
::execute() the same statement multiple times, which might trigger
sqlite3_reset()s at unexpected times (see, for instance, bug
#73530).
 [2019-11-08 21:37 UTC] svnpenn at gmail dot com
I am new to PHP SQLite so at first I was disappointed to find that "procedural style" functions were dropped with PHP SQLite3. Compare:

- https://php.net/function.sqlite-open
- https://php.net/sqlite3.construct

but I was going to give the Object oriented style a chance. Then I came across this bug here:

https://php.net/sqlite3stmt.execute#119404

and the example here still fails today:

https://3v4l.org/MMGio

I am surpised that its still open 6 years later. After realizing the poor PHP support for SQLite, I will simply call the SQLite binary going forward.
 [2020-01-31 21:20 UTC] antoni at friki dot cat
Hi,

today I've faced that problem.

Here my patch:
--- php-7.4.2/ext/sqlite3/sqlite3.c	2020-01-21 12:35:21.000000000 +0100
+++ php-7.4.2-sqlite3fixed/ext/sqlite3/sqlite3.c	2020-01-31 22:12:56.599653191 +0100
@@ -612,8 +612,11 @@
 	return_code = sqlite3_step(result->stmt_obj->stmt);
 
 	switch (return_code) {
-		case SQLITE_ROW: /* Valid Row */
 		case SQLITE_DONE: /* Valid but no results */
+                {
+			result->complete = 1;
+                }
+		case SQLITE_ROW: /* Valid Row */
 		{
 			php_sqlite3_free_list *free_item;
 			free_item = emalloc(sizeof(php_sqlite3_free_list));
@@ -624,6 +627,7 @@
 			break;
 		}
 		default:
+			result->complete = 1;
 			if (!EG(exception)) {
 				php_sqlite3_error(db_obj, "Unable to execute statement: %s", sqlite3_errmsg(db_obj->db));
 			}
@@ -713,6 +717,9 @@
 	return_code = sqlite3_step(stmt);
 
 	switch (return_code) {
+	        php_sqlite3_result *result_obj;
+	        zval *object = ZEND_THIS;
+	        result_obj = Z_SQLITE3_RESULT_P(object);
 		case SQLITE_ROW: /* Valid Row */
 		{
 			if (!entire_row) {
@@ -730,6 +737,8 @@
 		}
 		case SQLITE_DONE: /* Valid but no results */
 		{
+			result_obj->complete = 1;
+
 			if (!entire_row) {
 				RETVAL_NULL();
 			} else {
@@ -738,10 +747,12 @@
 			break;
 		}
 		default:
-		if (!EG(exception)) {
-			php_sqlite3_error(db_obj, "Unable to execute statement: %s", sqlite3_errmsg(db_obj->db));
-		}
-		RETVAL_FALSE;
+		        result_obj->complete = 1;
+
+		        if (!EG(exception)) {
+		        	php_sqlite3_error(db_obj, "Unable to execute statement: %s", sqlite3_errmsg(db_obj->db));
+		        }
+		        RETVAL_FALSE;
 	}
 	sqlite3_finalize(stmt);
 }
@@ -1844,14 +1855,16 @@
 
 	return_code = sqlite3_step(stmt_obj->stmt);
 
+	sqlite3_reset(stmt_obj->stmt);
+	object_init_ex(return_value, php_sqlite3_result_entry);
+	result = Z_SQLITE3_RESULT_P(return_value);
 	switch (return_code) {
-		case SQLITE_ROW: /* Valid Row */
 		case SQLITE_DONE: /* Valid but no results */
+                {
+			result->complete = 1;
+                }
+		case SQLITE_ROW: /* Valid Row */
 		{
-			sqlite3_reset(stmt_obj->stmt);
-			object_init_ex(return_value, php_sqlite3_result_entry);
-			result = Z_SQLITE3_RESULT_P(return_value);
-
 			result->is_prepared_statement = 1;
 			result->db_obj = stmt_obj->db_obj;
 			result->stmt_obj = stmt_obj;
@@ -1861,9 +1874,11 @@
 			break;
 		}
 		case SQLITE_ERROR:
+			result->complete = 1;
 			sqlite3_reset(stmt_obj->stmt);
 
 		default:
+			result->complete = 1;
 			if (!EG(exception)) {
 				php_sqlite3_error(stmt_obj->db_obj, "Unable to execute statement: %s", sqlite3_errmsg(sqlite3_db_handle(stmt_obj->stmt)));
 			}
@@ -1939,6 +1954,10 @@
 		return;
 	}
 
+	if (result_obj->complete) {
+		RETURN_FALSE;
+	}
+
 	RETURN_LONG(sqlite3_column_count(result_obj->stmt_obj->stmt));
 }
 /* }}} */
@@ -1958,6 +1977,11 @@
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &column) == FAILURE) {
 		return;
 	}
+
+	if (result_obj->complete) {
+		RETURN_FALSE;
+	}
+
 	column_name = (char*) sqlite3_column_name(result_obj->stmt_obj->stmt, column);
 
 	if (column_name == NULL) {
@@ -2007,6 +2031,10 @@
 		return;
 	}
 
+        if (result_obj->complete==1) {
+		return;
+        }
+
 	ret = sqlite3_step(result_obj->stmt_obj->stmt);
 	switch (ret) {
 		case SQLITE_ROW:
@@ -2043,6 +2071,7 @@
 			break;
 
 		default:
+			result_obj->complete = 1;
 			php_sqlite3_error(result_obj->db_obj, "Unable to execute statement: %s", sqlite3_errmsg(sqlite3_db_handle(result_obj->stmt_obj->stmt)));
 	}
 }
 [2020-01-31 22:20 UTC] antoni at friki dot cat
Here an PHP example reproducing the weird issue.

Find some commented out alternatives to avoid problems with current php-sqlite3 implementation:
  https://pastebin.com/kCvb65FH

The key point using current SQLite3 version is not to call sqlite3_step() on a statement if previous sqlite3_step() over same statement returned !=SQLITE_ROW.

The key point using current PHP version is not to call fetchArray() on query/execute $result. Using if numColumns()!==0 you can figure out if you want to run fetchArray() data after execute()/query().

Regards,
 [2020-01-31 23:23 UTC] antoni at friki dot cat
I think that small patch should fix most problems on fetchArray() usage.
It's not a breaking change as no one expects re-run insert/update/create statements using a function to acquire a row of data. IMAO

--- php-7.4.2/ext/sqlite3/sqlite3.c	2020-01-21 12:35:21.000000000 +0100
+++ php-7.4.2-sqlite3fixed2/ext/sqlite3/sqlite3.c	2020-01-31 23:37:17.449123405 +0100
@@ -2007,6 +2007,10 @@ PHP_METHOD(sqlite3result, fetchArray)
 		return;
 	}
 
+	if (sqlite3_column_count(result_obj->stmt_obj->stmt) == 0) {
+                return;
+        }
+
 	ret = sqlite3_step(result_obj->stmt_obj->stmt);
 	switch (ret) {
 		case SQLITE_ROW:
 [2020-02-01 00:13 UTC] antoni at friki dot cat
The following patch has been added/updated:

Patch Name: fix64531-sqlite3-fetchArray-skipNoColumns
Revision:   1580516014
URL:        https://bugs.php.net/patch-display.php?bug=64531&patch=fix64531-sqlite3-fetchArray-skipNoColumns&revision=1580516014
 [2020-02-24 11:56 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: [POC] Fix #64531: SQLite3Result::fetchArray runs the query again
On GitHub:  https://github.com/php/php-src/pull/5204
Patch:      https://github.com/php/php-src/pull/5204.patch
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 10:01:30 2024 UTC