Deprecated the concept of the fetch mode

[morozov]

Q	A
Type	feature, deprecation
BC Break	no

Proposed changes:

1. The FetchMode class, the UnknownFetchMode exception and the setFetchMode() methods are deprecated as well as the entire concept of the fetch mode.
2. The fetch() method is deprecated in favor of fetchNumeric(), fetchAssociative() and fetchOne() (like fetchOne() in ZF1).
3. The fetchAll() method is deprecated in favor of fetchAllNumeric() and fetchAllAssociative(). There's no currently replacement for fetchAll(FETCH_MODE::COLUMN). In a future major version, fetchColumn() will be used as a replacement (like fetchCol() in ZF1).
4. Connection::fetchAssoc() and ::fetchArray() are deprecated in favor of ::fetchAssociative() and ::fetchNumeric() respectively for consistency.
5. In order to make the Driver and the DBAL APIs distinct, two forward-compatible result statement interfaces are introduced. The one at the DBAL level extends the one at the Driver level (therefore, it can gain new DBAL-level features w/o having to implement them at the driver level).
6. Iteration over statements is deprecated. Only DBAL-level statements may be iterated via iterateNumeric(), iterateAssociative() or iterateColumn().
7. The StatementIterator class is deprecated.
8. Fetching in the “mixed”/“both” mode is deprecated.

See #4007.

TODO:

• See if the deprecated methods can be partially reimplemented using the new ones (the new ones don't support mixed fetch mode). Too much work for throwaway code.
• Add tests for the newly added APIs.
• Add tests to ensure that forward-incompatible implementations still work.
• Update slevomat/coding-standard in order to avoid false detection of the code like the following (cannot find the corresponding fixed issue):

  if ($condition) {
      yield from [];
  } else {
      yield from [];
  }

The existing tests are changed to the necessary minimum (only the expected method invocations) and do not represent intentional breaking changes.

[greg0ire]

🤔 what's the point of that second argument? It's always true, and the first argument is declared as array in the signature

[morozov]

It was needed before the introduction of FetchUtils to implement the "column" fetch modes. It's no longer needed, I’ll remove it.

[morozov]

Here's some food for thought:

dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php

Lines 281 to 283 in 115f20e

	private function iterate(int $mode, ...$arguments) : Traversable
	{
	parent::setFetchMode($mode, ...$arguments);

1. We're getting rid of the fetch mode as such and the APIs that allow setting it as a shared state.
2. We're introducing iterate*() methods at the driver statement level because some drivers natively implement it.
3. In fact, PDO is the only such driver, and "natively" here is a stretch since PDO itself is a wrapper. Being able to leverage this driver capability requires relying on the shared state that we want to avoid.

With that said, I think we should not introduce iterate* methods at the driver level and leave them only at the DBAL level.

[derrabus]

There's no currently replacement for fetchAll(FETCH_MODE::COLUMN).

If I had such a piece of code in my application and wanted to remove calls to deprecated functionality, what would be my upgrade path here?

[morozov]

If I had such a piece of code in my application and wanted to remove calls to deprecated functionality, what would be my upgrade path here?

There's no immediate upgrade path. You'll have to keep using the deprecated API until the next major release where it will be implemented by fetchColumn().

[greg0ire]

@morozov I didn't spot this issue, is that why you suggested to name the new method fetchCol()? It's not a perfect name because of the abbrevation, but it will make the upgrade experience better (and maybe we can then migrate to fetchColumn() in 4.x)

[tswcode]

I just stumbled upon the deprecated methods.
Everything is cleary documentated, but i cannot find the special PDO fetch method for "FETCH_KEY_PAIR".
Let's assume a sql, summarizing entity ids and a special count. Then I need the workflow like the old one:

$counts = $sqlStatement->fetchAll(PDO::FETCH_KEY_PAIR, 0); $sqlStatement->free();

The result has been an array with the ids as keys and counts as values.
Will you include this in the future or do I have to do implement my own method for this?

[morozov]

@tswcode PDO::FETCH_KEY_PAIR was never supported by DBAL. It was and is supported by PDO but not by other drivers. I was going to start implementing it but then realized that it's just as simple as:

foreach ($statement->iterateNumeric() as [$key => $value]) {
    // your code goes here
}

At the same time, implementing it in the DBAL will require it to handle the error when the resulting record set contains fewer than two columns (not sure how PDO handles that). If it's implemented in the userland, it's clear that the user should do the error handling.

Is there a case when the above doesn't work for you and you need it implemented in DBAL?

[morozov]

$conn = new PDO('sqlite::memory:');
$stmt = $conn->query('SELECT 1');

$stmt->fetchAll(PDO::FETCH_KEY_PAIR);

// PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: PDO::FETCH_KEY_PAIR fetch mode requires the result set to contain extactly 2 columns

Looks like PDO is strict enough about the number of columns. I think we can implement this in DBAL at the wrapper level which will automatically cover all supported drivers.

[tswcode]

@morozov Indeed it could be implemented in userland, yet I always try to avoid boilerplate code, when using this fetch mode. That would be great, if the DBAL would provide a method, so that i can replace the deprecated one.

[morozov]

@tswcode could you file a feature request please?

[tmotyl]

Btw, do you consider providing rector rule to help project migrate to doctrine v3 where the deprecated methods will be dropped?

[morozov]

No, but it would be nice if somebody kicked that off. As far as I understand, Rector has some rules for other Doctrine projects, so the development would have to be done in its repo.

[jokaorgua]

Could anyone please advise how to change current deprecated code without losing functionality?

$query = $this->connection->createQueryBuilder()
    ->select()....

$result = $query->execute()->fetchAll(FetchMode::STANDARD_OBJECT);

how to fetch an array of objects after this merge?

[derrabus]

$result = array_map(
    static fn (array $row) => (object) $row,
    $query->execute()->fetchAllAssociative()
);

[jokaorgua]

$result = array_map(
    static fn (array $row) => (object) $row,
    $query->execute()->fetchAllAssociative()
);

thanks. but this seems like a hack. are there any docs where it can be found why developers decided to remove the ability to fetch objects?

[morozov]

@jokaorgua please see #3070.

[derrabus]

thanks. but this seems like a hack.

It is a hack to unblock codebases that rely on object fetching. In the long run, that code should be migrated to use associative arrays instead.