[4.0] Database iterator broken compared to Joomla 3

[roland-d]

Steps to reproduce the issue

I found this issue while testing my RO CSVI extension on Joomla 4.

Function to use:

public function checkDatabase()
{
    $db    = $this->getDbo();

    $query = $db->getQuery(true)
        ->select($db->quoteName('id'))
        ->from($db->quoteName('#__content'));
    $db->setQuery($query);
    $records = $db->getIterator();
    $items = $records->count();

    $query->clear()
        ->select($db->quoteName('id'))
        ->from($db->quoteName('#__categories'));
    $db->setQuery($query);
    $db->loadResult();

    return $items === $records->count();
}

1. Run the following function in Joomla 3
2. The function will return true
3. Run the same function in Joomla 4
4. The function will return false

The extra query after the iterator query is required as that is how my extension works. What happens is that a user runs an export of articles, while the articles are being exported other queries are run for example to get a category path. In Joomla 4 this breaks the export but works fine in Joomla 3.

Expected result

The function to return true

Actual result

The function return false

System information (as much as possible)

Joomla 4.0-dev branch MySQL 5.7.29 PHP 7.4.19 Database type MySQLi or MySQL PDO does not make a difference

Additional comments

Both Joomla 3 and Joomla 4 instances are running the same webserver and same database server.

[HLeithner]

The database driver frees the result on setQuery() and calls closeCursor() on the current statement...

https://github.com/joomla-framework/database/blob/2.0-dev/src/DatabaseDriver.php#L1848

https://github.com/joomla-framework/database/blob/2.0-dev/src/DatabaseDriver.php#L777

I would expect that this is the reason, this doesn't happend in jdatabase 1.0

[wilsonge]

This does happen in the master branch of framework too - comes from https://github.com/joomla-framework/database/pull/38/files - but indeed doesn't in the CMS Database package. It should be noted I guess that currently the getIterator method in the CMS however never calls freeResult - so we actually have somewhat of a memory leak (obviously just in the context of the request but...)

[HLeithner]

That's possible but there is no reason to close the statement cursor.

You can unset the referenced statement if you return an iterator.

In this function you can unset the $statement variable and only provide the $statement variable to the getIterator function.

https://github.com/joomla-framework/database/blob/5ab075ba585a74d0c26dc74c547fa1f2dcbb7393/src/DatabaseDriver.php#L1071

  $statement = $this->statement;
  $this->statement = null;
  return $this->factory->getIterator($this->name, $statement, $column, $class);

The DatabaseIterator automatically closes the courser on destruction and Since the Statment is only referenced outside the Database Library it will destroyed by php as soons as the last variable reference is unset. @roland-d already tested this in a simple sample.

[wilsonge]

  $statement = $this->statement;
  $this->freeResult();
  return $this->factory->getIterator($this->name, $statement, $column, $class);

I guess this is better. But sure if that works it's fine. I just don't think we need to remove the code from setQuery.