ScalarColumnHydrator: prevent early-bail on falsy values

[MrMitch]

Hi !

This is my first contribution to Doctrine, please let me know if there is anything I can do to facilitate the review process.

--

Fix #9230

Falsy values stop the hydration process in $query->getSingleColumnResult(). If the array of values fetched from the database contains a value that is "falsy", every value after that value and including that value is missing from the array that is returned.

This is because of the way the database values are retreived in ScalarColumnHydrator :

orm/lib/Doctrine/ORM/Internal/Hydration/ScalarColumnHydrator.php

Lines 31 to 33 in f4d5283

	while ($row = $this->statement()->fetchOne()) {
	$result[] = $row;
	}

This PR fixes that by fetching all the values and returning them as is. As before, and to keep things consistent with the previous behavior of both ScalarHydrator and ScalarColumnHydrator, no type conversion is done between the database value and the PHP value. According to a comment in AbstractHydrator, this is the expected behavior for scalar hydrators that is a legacy behavior from 2.0 :

orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php

Lines 498 to 505 in f4d5283

	// WARNING: BC break! We know this is the desired behavior to type convert values, but this
	// erroneous behavior exists since 2.0 and we're forced to keep compatibility.
	if (! isset($cacheKeyInfo['isScalar'])) {
	$type = $cacheKeyInfo['type'];
	$value = $type ? $type->convertToPHPValue($value, $this->_platform) : $value;
	$fieldName = $cacheKeyInfo['dqlAlias'] . '_' . $fieldName;
	}

[derrabus]

Your patch introduces some complexity to the codebase by duplicating logic from the DBAL. Naive question: Shouldn't your problem be fixed if you changed the loop this way?

-         while ($row = $this->statement()->fetchOne()) {
+         foreach ($this->statement()->iterateColumn() as $row) {

[MrMitch]

@derrabus Thank you for your quick reply. You're absolutely right, using a simple foreach loop produces the same result and reduces the complexity, I've amended the code accordingly.

We could even further reduce the length of the method by using return iterator_to_array($this->statement()->iterateColumn()), but I'm not sure that would add real value or maintainability.

[MrMitch]

@derrabus I've amended the code further to fix support for databases that have native support for boolean values (like PostgreSQL).

[derrabus]

Thank you, @MrMitch!

[MrMitch]

Happy to contribute, thank you for reviewing this with care.