Browse Source

Remove results buffering and drop BufferredStatement.

ADmad 4 years ago
parent
commit
f30cdd6786

+ 0 - 27
src/Database/Driver/Mysql.php

@@ -17,11 +17,8 @@ declare(strict_types=1);
 namespace Cake\Database\Driver;
 
 use Cake\Database\Driver;
-use Cake\Database\Query;
 use Cake\Database\Schema\MysqlSchemaDialect;
 use Cake\Database\Schema\SchemaDialect;
-use Cake\Database\Statement\MysqlStatement;
-use Cake\Database\StatementInterface;
 use PDO;
 
 /**
@@ -179,30 +176,6 @@ class Mysql extends Driver
     }
 
     /**
-     * Prepares a sql statement to be executed
-     *
-     * @param \Cake\Database\Query|string $query The query to prepare.
-     * @return \Cake\Database\StatementInterface
-     */
-    public function prepare(Query|string $query): StatementInterface
-    {
-        $this->connect();
-        $isObject = $query instanceof Query;
-        /**
-         * @psalm-suppress PossiblyInvalidMethodCall
-         * @psalm-suppress PossiblyInvalidArgument
-         */
-        $statement = $this->_connection->prepare($isObject ? $query->sql() : $query);
-        $result = new MysqlStatement($statement, $this);
-        /** @psalm-suppress PossiblyInvalidMethodCall */
-        if ($isObject && $query->isBufferedResultsEnabled() === false) {
-            $result->bufferResults(false);
-        }
-
-        return $result;
-    }
-
-    /**
      * @inheritDoc
      */
     public function schemaDialect(): SchemaDialect

+ 2 - 13
src/Database/Driver/Sqlite.php

@@ -24,7 +24,6 @@ use Cake\Database\QueryCompiler;
 use Cake\Database\Schema\SchemaDialect;
 use Cake\Database\Schema\SqliteSchemaDialect;
 use Cake\Database\SqliteCompiler;
-use Cake\Database\Statement\PDOStatement;
 use Cake\Database\Statement\SqliteStatement;
 use Cake\Database\StatementInterface;
 use InvalidArgumentException;
@@ -188,19 +187,9 @@ class Sqlite extends Driver
     public function prepare(Query|string $query): StatementInterface
     {
         $this->connect();
-        $isObject = $query instanceof Query;
-        /**
-         * @psalm-suppress PossiblyInvalidMethodCall
-         * @psalm-suppress PossiblyInvalidArgument
-         */
-        $statement = $this->_connection->prepare($isObject ? $query->sql() : $query);
-        $result = new SqliteStatement(new PDOStatement($statement, $this), $this);
-        /** @psalm-suppress PossiblyInvalidMethodCall */
-        if ($isObject && $query->isBufferedResultsEnabled() === false) {
-            $result->bufferResults(false);
-        }
+        $statement = $this->_connection->prepare($query instanceof Query ? $query->sql() : $query);
 
-        return $result;
+        return new SqliteStatement($statement, $this);
     }
 
     /**

+ 7 - 9
src/Database/Driver/Sqlserver.php

@@ -191,10 +191,6 @@ class Sqlserver extends Driver
         $this->connect();
 
         $sql = $query;
-        $options = [
-            PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL,
-            PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE => PDO::SQLSRV_CURSOR_BUFFERED,
-        ];
         if ($query instanceof Query) {
             $sql = $query->sql();
             if (count($query->getValueBinder()->bindings()) > 2100) {
@@ -205,14 +201,16 @@ class Sqlserver extends Driver
                     'If using an Association, try changing the `strategy` from select to subquery.'
                 );
             }
-
-            if (!$query->isBufferedResultsEnabled()) {
-                $options = [];
-            }
         }
 
         /** @psalm-suppress PossiblyInvalidArgument */
-        $statement = $this->_connection->prepare($sql, $options);
+        $statement = $this->_connection->prepare(
+            $sql,
+            [
+                PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL,
+                PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE => PDO::SQLSRV_CURSOR_BUFFERED,
+            ]
+        );
 
         return new SqlserverStatement($statement, $this);
     }

+ 0 - 62
src/Database/Query.php

@@ -171,14 +171,6 @@ class Query implements ExpressionInterface, IteratorAggregate, Stringable
     protected ?FunctionsBuilder $_functionsBuilder = null;
 
     /**
-     * Boolean for tracking whether buffered results
-     * are enabled.
-     *
-     * @var bool
-     */
-    protected bool $_useBufferedResults = true;
-
-    /**
      * The Type map for fields in the select clause
      *
      * @var \Cake\Database\TypeMap|null
@@ -2169,60 +2161,6 @@ class Query implements ExpressionInterface, IteratorAggregate, Stringable
     }
 
     /**
-     * Enables/Disables buffered results.
-     *
-     * When enabled the results returned by this Query will be
-     * buffered. This enables you to iterate a result set multiple times, or
-     * both cache and iterate it.
-     *
-     * When disabled it will consume less memory as fetched results are not
-     * remembered for future iterations.
-     *
-     * @param bool $enable Whether to enable buffering
-     * @return $this
-     */
-    public function enableBufferedResults(bool $enable = true)
-    {
-        $this->_dirty();
-        $this->_useBufferedResults = $enable;
-
-        return $this;
-    }
-
-    /**
-     * Disables buffered results.
-     *
-     * Disabling buffering will consume less memory as fetched results are not
-     * remembered for future iterations.
-     *
-     * @return $this
-     */
-    public function disableBufferedResults()
-    {
-        $this->_dirty();
-        $this->_useBufferedResults = false;
-
-        return $this;
-    }
-
-    /**
-     * Returns whether buffered results are enabled/disabled.
-     *
-     * When enabled the results returned by this Query will be
-     * buffered. This enables you to iterate a result set multiple times, or
-     * both cache and iterate it.
-     *
-     * When disabled it will consume less memory as fetched results are not
-     * remembered for future iterations.
-     *
-     * @return bool
-     */
-    public function isBufferedResultsEnabled(): bool
-    {
-        return $this->_useBufferedResults;
-    }
-
-    /**
      * Sets the TypeMap class where the types for each of the fields in the
      * select clause are stored.
      *

+ 0 - 45
src/Database/Statement/BufferResultsTrait.php

@@ -1,45 +0,0 @@
-<?php
-declare(strict_types=1);
-
-/**
- * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
- * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- *
- * Licensed under The MIT License
- * For full copyright and license information, please see the LICENSE.txt
- * Redistributions of files must retain the above copyright notice.
- *
- * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- * @link          https://cakephp.org CakePHP(tm) Project
- * @since         3.0.0
- * @license       https://opensource.org/licenses/mit-license.php MIT License
- */
-namespace Cake\Database\Statement;
-
-/**
- * Contains a setter for marking a Statement as buffered
- *
- * @internal
- */
-trait BufferResultsTrait
-{
-    /**
-     * Whether to buffer results in php
-     *
-     * @var bool
-     */
-    protected bool $_bufferResults = true;
-
-    /**
-     * Whether to buffer results in php
-     *
-     * @param bool $buffer Toggle buffering
-     * @return $this
-     */
-    public function bufferResults(bool $buffer)
-    {
-        $this->_bufferResults = $buffer;
-
-        return $this;
-    }
-}

+ 0 - 335
src/Database/Statement/BufferedStatement.php

@@ -1,335 +0,0 @@
-<?php
-declare(strict_types=1);
-
-/**
- * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
- * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- *
- * Licensed under The MIT License
- * For full copyright and license information, please see the LICENSE.txt
- * Redistributions of files must retain the above copyright notice.
- *
- * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- * @link          https://cakephp.org CakePHP(tm) Project
- * @since         3.0.0
- * @license       https://opensource.org/licenses/mit-license.php MIT License
- */
-namespace Cake\Database\Statement;
-
-use Cake\Database\DriverInterface;
-use Cake\Database\StatementInterface;
-use Cake\Database\TypeConverterTrait;
-use Iterator;
-use RuntimeException;
-
-/**
- * A statement decorator that implements buffered results.
- *
- * This statement decorator will save fetched results in memory, allowing
- * the iterator to be rewound and reused.
- */
-class BufferedStatement implements Iterator, StatementInterface
-{
-    use TypeConverterTrait;
-
-    /**
-     * If true, all rows were fetched
-     *
-     * @var bool
-     */
-    protected bool $_allFetched = false;
-
-    /**
-     * The decorated statement
-     *
-     * @var \Cake\Database\StatementInterface
-     * @phpcsSuppress SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingNativeTypeHint
-     */
-    protected $statement;
-
-    /**
-     * The driver for the statement
-     *
-     * @var \Cake\Database\DriverInterface
-     */
-    protected DriverInterface $_driver;
-
-    /**
-     * The in-memory cache containing results from previous iterators
-     *
-     * @var array<int, array>
-     */
-    protected array $buffer = [];
-
-    /**
-     * Whether this statement has already been executed
-     *
-     * @var bool
-     */
-    protected bool $_hasExecuted = false;
-
-    /**
-     * The current iterator index.
-     *
-     * @var int
-     */
-    protected int $index = 0;
-
-    /**
-     * Constructor
-     *
-     * @param \Cake\Database\StatementInterface $statement Statement implementation such as PDOStatement
-     * @param \Cake\Database\DriverInterface $driver Driver instance
-     */
-    public function __construct(StatementInterface $statement, DriverInterface $driver)
-    {
-        $this->statement = $statement;
-        $this->_driver = $driver;
-    }
-
-    /**
-     * Magic getter to return $queryString as read-only.
-     *
-     * @param string $property internal property to get
-     * @return string|null
-     */
-    public function __get(string $property): mixed
-    {
-        if ($property === 'queryString') {
-            /** @psalm-suppress NoInterfaceProperties */
-            return $this->statement->queryString;
-        }
-
-        throw new RuntimeException("Cannot access undefined property `$property`.");
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function bindValue($column, $value, $type = 'string'): void
-    {
-        $this->statement->bindValue($column, $value, $type);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function closeCursor(): void
-    {
-        $this->statement->closeCursor();
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function columnCount(): int
-    {
-        return $this->statement->columnCount();
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function errorCode(): string|int
-    {
-        return $this->statement->errorCode();
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function errorInfo(): array
-    {
-        return $this->statement->errorInfo();
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function execute(?array $params = null): bool
-    {
-        $this->_reset();
-        $this->_hasExecuted = true;
-
-        return $this->statement->execute($params);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function fetchColumn(int $position): mixed
-    {
-        $result = $this->fetch(static::FETCH_TYPE_NUM);
-        if ($result !== false && isset($result[$position])) {
-            return $result[$position];
-        }
-
-        return false;
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function bind(array $params, array $types): void
-    {
-        $this->statement->bind($params, $types);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function lastInsertId(?string $table = null, ?string $column = null): string|int
-    {
-        return $this->statement->lastInsertId($table, $column);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function fetch(string|int $type = self::FETCH_TYPE_NUM): mixed
-    {
-        if ($this->_allFetched) {
-            $row = false;
-            if (isset($this->buffer[$this->index])) {
-                $row = $this->buffer[$this->index];
-            }
-            $this->index += 1;
-
-            if ($row && $type === static::FETCH_TYPE_NUM) {
-                return array_values($row);
-            }
-
-            return $row;
-        }
-
-        $record = $this->statement->fetch($type);
-        if ($record === false) {
-            $this->_allFetched = true;
-            $this->statement->closeCursor();
-
-            return false;
-        }
-        $this->buffer[] = $record;
-
-        return $record;
-    }
-
-    /**
-     * @return array
-     */
-    public function fetchAssoc(): array
-    {
-        $result = $this->fetch(static::FETCH_TYPE_ASSOC);
-
-        return $result ?: [];
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function fetchAll($type = self::FETCH_TYPE_NUM): array|false
-    {
-        if ($this->_allFetched) {
-            return $this->buffer;
-        }
-        $results = $this->statement->fetchAll($type);
-        if ($results !== false) {
-            $this->buffer = array_merge($this->buffer, $results);
-        }
-        $this->_allFetched = true;
-        $this->statement->closeCursor();
-
-        return $this->buffer;
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function rowCount(): int
-    {
-        if (!$this->_allFetched) {
-            $this->fetchAll(static::FETCH_TYPE_ASSOC);
-        }
-
-        return count($this->buffer);
-    }
-
-    /**
-     * Reset all properties
-     *
-     * @return void
-     */
-    protected function _reset(): void
-    {
-        $this->buffer = [];
-        $this->_allFetched = false;
-        $this->index = 0;
-    }
-
-    /**
-     * Returns the current key in the iterator
-     *
-     * @return mixed
-     */
-    public function key(): mixed
-    {
-        return $this->index;
-    }
-
-    /**
-     * Returns the current record in the iterator
-     *
-     * @return mixed
-     */
-    public function current(): mixed
-    {
-        return $this->buffer[$this->index];
-    }
-
-    /**
-     * Rewinds the collection
-     *
-     * @return void
-     */
-    public function rewind(): void
-    {
-        $this->index = 0;
-    }
-
-    /**
-     * Returns whether the iterator has more elements
-     *
-     * @return bool
-     */
-    public function valid(): bool
-    {
-        $old = $this->index;
-        $row = $this->fetch(self::FETCH_TYPE_ASSOC);
-
-        // Restore the index as fetch() increments during
-        // the cache scenario.
-        $this->index = $old;
-
-        return $row !== false;
-    }
-
-    /**
-     * Advances the iterator pointer to the next element
-     *
-     * @return void
-     */
-    public function next(): void
-    {
-        $this->index += 1;
-    }
-
-    /**
-     * Get the wrapped statement
-     *
-     * @return \Cake\Database\StatementInterface
-     */
-    public function getInnerStatement(): StatementInterface
-    {
-        return $this->statement;
-    }
-}

+ 0 - 46
src/Database/Statement/MysqlStatement.php

@@ -1,46 +0,0 @@
-<?php
-declare(strict_types=1);
-
-/**
- * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
- * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- *
- * Licensed under The MIT License
- * For full copyright and license information, please see the LICENSE.txt
- * Redistributions of files must retain the above copyright notice.
- *
- * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- * @link          https://cakephp.org CakePHP(tm) Project
- * @since         3.0.0
- * @license       https://opensource.org/licenses/mit-license.php MIT License
- */
-namespace Cake\Database\Statement;
-
-use PDO;
-
-/**
- * Statement class meant to be used by a MySQL PDO driver
- *
- * @internal
- */
-class MysqlStatement extends PDOStatement
-{
-    use BufferResultsTrait;
-
-    /**
-     * @inheritDoc
-     */
-    public function execute(?array $params = null): bool
-    {
-        $connection = $this->_driver->getConnection();
-
-        try {
-            $connection->setAttribute(PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, $this->_bufferResults);
-            $result = $this->_statement->execute($params);
-        } finally {
-            $connection->setAttribute(PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, true);
-        }
-
-        return $result;
-    }
-}

+ 1 - 19
src/Database/Statement/SqliteStatement.php

@@ -21,26 +21,8 @@ namespace Cake\Database\Statement;
  *
  * @internal
  */
-class SqliteStatement extends StatementDecorator
+class SqliteStatement extends PDOStatement
 {
-    use BufferResultsTrait;
-
-    /**
-     * @inheritDoc
-     */
-    public function execute(?array $params = null): bool
-    {
-        if ($this->_statement instanceof BufferedStatement) {
-            $this->_statement = $this->_statement->getInnerStatement();
-        }
-
-        if ($this->_bufferResults) {
-            $this->_statement = new BufferedStatement($this->_statement, $this->_driver);
-        }
-
-        return $this->_statement->execute($params);
-    }
-
     /**
      * Returns the number of rows returned of affected by last execution
      *

+ 1 - 3
src/ORM/Behavior/Translate/EavStrategy.php

@@ -294,7 +294,6 @@ class EavStrategy implements TranslateStrategyInterface
                     'foreign_key' => $key,
                     'model' => $model,
                 ])
-                ->disableBufferedResults()
                 ->all()
                 ->indexBy('field');
         }
@@ -499,8 +498,7 @@ class EavStrategy implements TranslateStrategyInterface
         $query = $association->find()
             ->select(['id', 'num' => 0])
             ->where(current($ruleSet))
-            ->disableHydration()
-            ->disableBufferedResults();
+            ->disableHydration();
 
         unset($ruleSet[0]);
         foreach ($ruleSet as $i => $conditions) {

+ 0 - 1
src/ORM/Behavior/Translate/ShadowTableStrategy.php

@@ -403,7 +403,6 @@ class ShadowTableStrategy implements TranslateStrategyInterface
             $translation = $this->translationTable->find()
                 ->select(array_merge(['id', 'locale'], $fields))
                 ->where($where)
-                ->disableBufferedResults()
                 ->first();
         }
 

+ 1 - 2
src/ORM/Query.php

@@ -1334,7 +1334,6 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
 
         return parent::__debugInfo() + [
             'hydrate' => $this->_hydrate,
-            'buffered' => $this->_useBufferedResults,
             'formatters' => count($this->_formatters),
             'mapReducers' => count($this->_mapReduce),
             'contain' => $eagerLoader->getContain(),
@@ -1407,7 +1406,7 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
     {
         $result = $this->_applyDecorators($result);
 
-        if (!($result instanceof ResultSet) && $this->isBufferedResultsEnabled()) {
+        if (!($result instanceof ResultSet)) {
             $class = $this->_decoratorClass();
             $result = new $class($result->buffered());
         }

+ 0 - 26
tests/TestCase/Database/ConnectionTest.php

@@ -17,12 +17,10 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Database;
 
 use Cake\Cache\Engine\NullEngine;
-use Cake\Collection\Collection;
 use Cake\Core\App;
 use Cake\Database\Connection;
 use Cake\Database\Driver;
 use Cake\Database\Driver\Mysql;
-use Cake\Database\Driver\Sqlite;
 use Cake\Database\Driver\Sqlserver;
 use Cake\Database\Exception\MissingConnectionException;
 use Cake\Database\Exception\MissingDriverException;
@@ -275,30 +273,6 @@ class ConnectionTest extends TestCase
     }
 
     /**
-     * test executing a buffered query interacts with Collection well.
-     */
-    public function testBufferedStatementCollectionWrappingStatement(): void
-    {
-        $this->skipIf(
-            !($this->connection->getDriver() instanceof Sqlite),
-            'Only required for SQLite driver which does not support buffered results natively'
-        );
-
-        $statement = $this->connection->query('SELECT * FROM things LIMIT 3');
-
-        $collection = new Collection($statement);
-        $result = $collection->extract('id')->toArray();
-        $this->assertEquals(['1', '2'], $result);
-
-        // Check iteration after extraction
-        $result = [];
-        foreach ($collection as $v) {
-            $result[] = $v['id'];
-        }
-        $this->assertEquals(['1', '2'], $result);
-    }
-
-    /**
      * Tests that passing a unknown value to a query throws an exception
      */
     public function testExecuteWithMissingType(): void

+ 5 - 4
tests/TestCase/Database/QueryTest.php

@@ -4125,9 +4125,11 @@ class QueryTest extends TestCase
             ->select(['name'])
             ->from(['authors'])
             ->where(['name IS NOT' => 'larry'])
-            ->execute();
-        $this->assertCount(3, $results->fetchAll());
-        $this->assertNotEquals(['name' => 'larry'], $results->fetch('assoc'));
+            ->execute()
+            ->fetchAll('assoc');
+
+        $this->assertCount(3, $results);
+        $this->assertNotEquals(['name' => 'larry'], $results[0]);
     }
 
     /**
@@ -4930,7 +4932,6 @@ class QueryTest extends TestCase
             ])
             ->from('profiles')
             ->limit(1)
-            ->enableBufferedResults(false)
             ->execute();
         $results = $stmt->fetch(StatementDecorator::FETCH_TYPE_OBJ);
         $stmt->closeCursor();

+ 1 - 4
tests/TestCase/ORM/QueryTest.php

@@ -1177,8 +1177,7 @@ class QueryTest extends TestCase
         $query = new Query($this->connection, $table);
         $query->select(['id']);
 
-        $first = $query->enableHydration(false)
-            ->enableBufferedResults(false)->first();
+        $first = $query->enableHydration(false)->first();
 
         $this->assertEquals(['id' => 1], $first);
     }
@@ -2530,7 +2529,6 @@ class QueryTest extends TestCase
         $table->hasMany('articles');
         $query = $table->find()
             ->where(['id > ' => 1])
-            ->enableBufferedResults(false)
             ->enableHydration(false)
             ->matching('articles')
             ->applyOptions(['foo' => 'bar'])
@@ -2570,7 +2568,6 @@ class QueryTest extends TestCase
             'decorators' => 0,
             'executed' => false,
             'hydrate' => false,
-            'buffered' => false,
             'formatters' => 1,
             'mapReducers' => 1,
             'contain' => [],

+ 0 - 1
tests/TestCase/ORM/ResultSetTest.php

@@ -431,7 +431,6 @@ class ResultSetTest extends TestCase
         $this->assertCount(0, $messages);
 
         $results = $this->table->find('all')
-            ->enableBufferedResults()
             ->where(['id' => 0])
             ->all();