fixing excessive memory usage

[dima-stefantsov]

This fixes the issue #960

I am not sure the

if ( empty( $result_array ) || !is_array( $result_array ) ) {
    return array();
}

code in GetCol() is still required. I've left it be.

I am not sure 'fetchStyle' => \PDO::FETCH_COLUMN will work without the 2nd parameter, especially on databases other than mysql. It works for me.

I have tried changing runQuery() code to have a return instead of side-effectively setting a field,

it looked something like this

public function runQuery( $sql, $bindings, $options = array() )
{
    $this->connect();
    if ( $this->loggingEnabled && $this->logger ) {
        $this->logger->log( $sql, $bindings );
    }

    $result_array = null;
    try {
        if ( strpos( 'pgsql', $this->dsn ) === 0 ) {
            if (defined('\\PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT')) {
                        $statement = @$this->pdo->prepare($sql, array(\PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT => TRUE));
                    } else {
                        $statement = $this->pdo->prepare($sql);
                    }
        } else {
            $statement = $this->pdo->prepare( $sql );
        }
        $this->bindParams( $statement, $bindings );
        $statement->execute();
        $this->queryCounter ++;
        $this->affectedRows = $statement->rowCount();
        if ( isset( $options['noFetch'] ) && $options['noFetch'] ) {
            return $statement;
        }

        if ( $statement->columnCount() ) {
            $fetchStyle = ( isset( $options['fetchStyle'] ) ) ? $options['fetchStyle'] : NULL;
            if ( is_null( $fetchStyle) ) {
                $result_array = $statement->fetchAll();
            } else {
                $result_array = $statement->fetchAll( $fetchStyle );
            }

            if ( $this->loggingEnabled && $this->logger ) {
                $this->logger->log( 'resultset: ' . count( $result_array ) . ' rows' );
            }
        }
    } catch ( \PDOException $e ) {
        //Unfortunately the code field is supposed to be int by default (php)
        //So we need a property to convey the SQL State code.
        $err = $e->getMessage();
        if ( $this->loggingEnabled && $this->logger ) $this->logger->log( 'An error occurred: ' . $err );
        $exception = new SQL( $err, 0, $e );
        $exception->setSQLState( $e->getCode() );
        $exception->setDriverDetails( $e->errorInfo );
        throw $exception;
    }

    return $result_array === null ?
        array() :
        $result_array;
}

, but it is already a mess anyway, with multiple returns of different types and severe depth, so I decided to not touch it.

I am not sure this code runs and passes tests, as I have not compiled and ran tests on it.

I have edited rb-mysql.php inline though, and it works excellently. Fast and x5+ less RAM.

[Lynesth]

I think this should be 2 different PRs.

One to use FETCH_COLUMN in getCol ;
And one to properly either:

• add an option that will null resultArray everywhere it's returned (like what you did in the changes you suggest).
• rewrite runQuery so that it returns the result directly. This can even still store the rows in a resultArray if some option is enabled, for backward compatibility (that's assuming anyone is actually using that somewhere).

On another note, regarding your specific use case, you might benefit from using a cursor instead of actually dumping all 3M results in an array.

[gabordemooij]

Thanks, very nice PR. I will request some more credits for CI/CD to have it tested.

[gabordemooij]

Additional tokens for 2025 have been requested at travis-ci.

[gabordemooij]

Hm, there seems to be a bit of trouble with travis. Normally travis-ci would automatically run incoming prs.

[gabordemooij]

Sorry for the minor edit, it was the only way to trigger a build. Additionally, the extra newline makes the code a bit more readable anyway.