Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Additional changes based on the discussion in #4007 #4034

Merged
merged 4 commits into from
May 28, 2020
Merged

Additional changes based on the discussion in #4007 #4034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0421d1
Update documentation to use the new fetch API
morozov May 9, 2020
4af4999
ResultCacheStatement::fetchAllNumeric() should return numeric arrays
morozov May 9, 2020
3eb6f0a
Refactored ResultCacheTest for better coverage
morozov May 27, 2020
e74b97c
ResultCacheStatement should always fetch in associative mode
morozov May 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/en/reference/caching.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object is closed:

<?php
$stmt = $conn->executeCacheQuery($query, $params, $types, new QueryCacheProfile(0, "some key"));
$data = $stmt->fetchAll();
$data = $stmt->fetchAllAssociative();
$stmt->closeCursor(); // at this point the result is cached

.. warning::
Expand Down
34 changes: 17 additions & 17 deletions docs/en/reference/data-retrieval-and-manipulation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ the query until there are no more rows:

<?php

while ($row = $stmt->fetch()) {
while (($row = $stmt->fetchAssociative()) !== false) {
echo $row['headline'];
}

Expand Down Expand Up @@ -308,7 +308,7 @@ Prepare a given SQL statement and return the
<?php
$statement = $conn->prepare('SELECT * FROM user');
$statement->execute();
$users = $statement->fetchAll();
$users = $statement->fetchAllAssociative();

/*
array(
Expand Down Expand Up @@ -346,7 +346,7 @@ parameters to the execute method, then returning the statement:

<?php
$statement = $conn->executeQuery('SELECT * FROM user WHERE username = ?', array('jwage'));
$user = $statement->fetch();
$user = $statement->fetchAssociative();

/*
array(
Expand All @@ -360,15 +360,15 @@ to perform necessary type conversions between actual input
parameters and expected database values. See the
:ref:`Types <mappingMatrix>` section for more information.

fetchAll()
~~~~~~~~~~
fetchAllAssociative()
~~~~~~~~~~~~~~~~~~~~~

Execute the query and fetch all results into an array:

.. code-block:: php

<?php
$users = $conn->fetchAll('SELECT * FROM user');
$users = $conn->fetchAllAssociative('SELECT * FROM user');

/*
array(
Expand All @@ -379,15 +379,15 @@ Execute the query and fetch all results into an array:
)
*/

fetchArray()
~~~~~~~~~~~~
fetchNumeric()
~~~~~~~~~~~~~~

Numeric index retrieval of first result row of the given query:

.. code-block:: php

<?php
$user = $conn->fetchArray('SELECT * FROM user WHERE username = ?', array('jwage'));
$user = $conn->fetchNumeric('SELECT * FROM user WHERE username = ?', array('jwage'));

/*
array(
Expand All @@ -396,26 +396,26 @@ Numeric index retrieval of first result row of the given query:
)
*/

fetchColumn()
~~~~~~~~~~~~~
fetchOne()
~~~~~~~~~~

Retrieve only the given column of the first result row.
Retrieve only the value of the first column of the first result row.

.. code-block:: php

<?php
$username = $conn->fetchColumn('SELECT username FROM user WHERE id = ?', array(1), 0);
$username = $conn->fetchOne('SELECT username FROM user WHERE id = ?', array(1), 0);
echo $username; // jwage

fetchAssoc()
~~~~~~~~~~~~
fetchAssociative()
~~~~~~~~~~~~~~~~~~

Retrieve assoc row of the first result row.
Retrieve associative array of the first result row.

.. code-block:: php

<?php
$user = $conn->fetchAssoc('SELECT * FROM user WHERE username = ?', array('jwage'));
$user = $conn->fetchAssociative('SELECT * FROM user WHERE username = ?', array('jwage'));
/*
array(
'username' => 'jwage',
Expand Down
47 changes: 26 additions & 21 deletions lib/Doctrine/DBAL/Cache/ResultCacheStatement.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use InvalidArgumentException;
use IteratorAggregate;
use PDO;
use function array_map;
use function array_merge;
use function array_values;
use function assert;
Expand Down Expand Up @@ -55,7 +56,7 @@ class ResultCacheStatement implements IteratorAggregate, ResultStatement, Forwar
*/
private $emptied = false;

/** @var mixed[] */
/** @var array<int,array<string,mixed>> */
private $data;

/** @var int */
Expand Down Expand Up @@ -179,18 +180,26 @@ public function fetch($fetchMode = null, $cursorOrientation = PDO::FETCH_ORI_NEX
*/
public function fetchAll($fetchMode = null, $fetchArgument = null, $ctorArgs = null)
{
$data = $this->statement->fetchAll($fetchMode, $fetchArgument, $ctorArgs);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the root cause of #4033.


if ($fetchMode === FetchMode::COLUMN) {
foreach ($data as $key => $value) {
$data[$key] = [$value];
}
}
$data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE, $fetchArgument, $ctorArgs);

$this->data = $data;
$this->emptied = true;

return $this->data;
if ($fetchMode === FetchMode::NUMERIC) {
foreach ($data as $i => $row) {
$data[$i] = array_values($row);
}
} elseif ($fetchMode === FetchMode::MIXED) {
foreach ($data as $i => $row) {
$data[$i] = array_merge($row, array_values($row));
}
} elseif ($fetchMode === FetchMode::COLUMN) {
foreach ($data as $i => $row) {
$data[$i] = reset($row);
}
}

return $data;
Comment on lines +188 to +202
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this:

Suggested change
if ($fetchMode === FetchMode::NUMERIC) {
foreach ($data as $i => $row) {
$data[$i] = array_values($row);
}
} elseif ($fetchMode === FetchMode::MIXED) {
foreach ($data as $i => $row) {
$data[$i] = array_merge($row, array_values($row));
}
} elseif ($fetchMode === FetchMode::COLUMN) {
foreach ($data as $i => $row) {
$data[$i] = reset($row);
}
}
return $data;
return array_map(function(array $row) use ($fetchMode) {
switch ($fetchMode)
{
case FetchMode::NUMERIC:
return array_values($row);
case FetchMode::MIXED:
return array_merge($row, array_values($row));
case FetchMode::COLUMN:
return reset($row);
default:
return $row;
}
}, $data);

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will require a switch on each row instead of an if/else once.

}

/**
Expand Down Expand Up @@ -247,7 +256,9 @@ public function fetchAllNumeric() : array
$data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE);
}

return $this->store($data);
$this->store($data);

return array_map('array_values', $this->data);
}

/**
Expand All @@ -261,7 +272,9 @@ public function fetchAllAssociative() : array
$data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE);
}

return $this->store($data);
$this->store($data);

return $this->data;
}

/**
Expand Down Expand Up @@ -311,19 +324,11 @@ private function doFetch()
}

/**
* @param array<int,array<mixed>> $data
*
* @return array<int,array<mixed>>
* @param array<int,array<string,mixed>> $data
*/
private function store(array $data) : array
private function store(array $data) : void
{
foreach ($data as $key => $value) {
$data[$key] = [$value];
}

$this->data = $data;
$this->emptied = true;

return $this->data;
}
}
42 changes: 22 additions & 20 deletions tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,13 @@ public function testMixingFetch() : void
$numExpectedResult[] = array_values($v);
}

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$data = $this->hydrateStmt($stmt, FetchMode::ASSOCIATIVE);

self::assertEquals($this->expectedResult, $data);

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$data = $this->hydrateStmt($stmt, FetchMode::NUMERIC);

Expand All @@ -124,26 +124,26 @@ public function testIteratorFetch() : void

private function assertStandardAndIteratorFetchAreEqual(int $fetchMode) : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));
$data = $this->hydrateStmt($stmt, $fetchMode);

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));
$dataIterator = $this->hydrateStmtIterator($stmt, $fetchMode);

self::assertEquals($data, $dataIterator);
}

public function testDontCloseNoCache() : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$data = [];

while ($row = $stmt->fetch(FetchMode::ASSOCIATIVE)) {
$data[] = $row;
}

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$data = [];

Expand All @@ -156,12 +156,12 @@ public function testDontCloseNoCache() : void

public function testDontFinishNoCache() : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$stmt->fetch(FetchMode::ASSOCIATIVE);
$stmt->closeCursor();

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

$this->hydrateStmt($stmt, FetchMode::NUMERIC);

Expand All @@ -171,7 +171,7 @@ public function testDontFinishNoCache() : void
public function testFetchAllAndFinishSavesCache() : void
{
$layerCache = new ArrayCache();
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(10, 'testcachekey', $layerCache));
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'testcachekey', $layerCache));
$stmt->fetchAll();
$stmt->closeCursor();

Expand Down Expand Up @@ -199,13 +199,13 @@ public function testFetchAllColumn() : void
*/
private function assertCacheNonCacheSelectSameFetchModeAreEqual(array $expectedResult, int $fetchMode) : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

self::assertEquals(2, $stmt->columnCount());
$data = $this->hydrateStmt($stmt, $fetchMode);
self::assertEquals($expectedResult, $data);

$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(10, 'testcachekey'));
$stmt = $this->connection->executeQuery('SELECT * FROM caching ORDER BY test_int ASC', [], [], new QueryCacheProfile(0, 'testcachekey'));

self::assertEquals(2, $stmt->columnCount());
$data = $this->hydrateStmt($stmt, $fetchMode);
Expand All @@ -215,23 +215,24 @@ private function assertCacheNonCacheSelectSameFetchModeAreEqual(array $expectedR

public function testEmptyResultCache() : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(10, 'emptycachekey'));
$data = $this->hydrateStmt($stmt);
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'emptycachekey'));
$this->hydrateStmt($stmt);

$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(10, 'emptycachekey'));
$data = $this->hydrateStmt($stmt);
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'emptycachekey'));
$this->hydrateStmt($stmt);

self::assertCount(1, $this->sqlLogger->queries, 'just one dbal hit');
}

public function testChangeCacheImpl() : void
{
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(10, 'emptycachekey'));
$data = $this->hydrateStmt($stmt);
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'emptycachekey'));
$this->hydrateStmt($stmt);

$secondCache = new ArrayCache();
$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(10, 'emptycachekey', $secondCache));
$data = $this->hydrateStmt($stmt);

$stmt = $this->connection->executeQuery('SELECT * FROM caching WHERE test_int > 500', [], [], new QueryCacheProfile(0, 'emptycachekey', $secondCache));
$this->hydrateStmt($stmt);

self::assertCount(2, $this->sqlLogger->queries, 'two hits');
self::assertCount(1, $secondCache->fetch('emptycachekey'));
Expand All @@ -243,7 +244,8 @@ public function testChangeCacheImpl() : void
private function hydrateStmt(ResultStatement $stmt, int $fetchMode = FetchMode::ASSOCIATIVE) : array
{
$data = [];
while ($row = $stmt->fetch($fetchMode)) {

foreach ($stmt->fetchAll($fetchMode) as $row) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how #4033 was discovered.

$data[] = is_array($row) ? array_change_key_case($row, CASE_LOWER) : $row;
}

Expand Down