PDOStatement::fetchObject() expects parameter 2 to be array, null given in Drupal\Core\Database\StatementWrapper->fetchObject()

Triaged as part of BSI, bug still exists in 11.x

I feel like the correct solution is array $constructor_arguments = [], but that may require a BC dance which it looks like these classes have already been through.

+++ b/core/lib/Drupal/Core/Database/StatementWrapperIterator.php
@@ -217,7 +217,7 @@ public function fetchAssoc() {
   public function fetchObject(string $class_name = NULL, array $constructor_arguments = NULL) {

I think it would be better to update fetchObject() to default to an empty array and trigger a deprecation warning if NULL is passed.

I agree (see #8), let's see what others think.

The last submitted patch, 7: 3259740-7--test-only.patch, failed testing. View results →

Think #9 would be the safest. A trigger_error if null is passed in.

Explicitly passing NULL produces a similar error, therefore does not need a deprecation because it never worked in the first place.

TypeError: PDOStatement::fetchObject(): Argument #2 ($constructorArgs) must be of type array, null given in PDOStatement->fetchObject()

Oh thanks for that @acbramley!

The last submitted patch, 7: 3259740-7--test-only.patch, failed testing. View results →

Going to ping daffie for a review

I think we need to add a deprecation warning when the second parameter is explicitly set to NULL and add testing for that.

I did the review, therefor I should have removed the tag.

I think we need to add a deprecation warning when the second parameter is explicitly set to NULL and add testing for that.

See #13 you can't explicitly pass NULL to the function, that fails with the same type error because the parameter is typed as an array.

Shouldn’t we fully align to the signature since we’re at it?

public PDOStatement::fetchObject(?string $class = "stdClass", array $constructorArgs = []): object|false

Maybe without the return typehint for now.

Not sure where we are with BC here - the interface is not typehinted yet, but concrete classes are and contrib extending from them would fail.

After talking to @acbramley and @larowlan on Slack, do I get their point. The parent method from PHP PDO also does not work when the second parameter is set to NULL. Therefor that is not a BC break.

Not sure where we are with BC here - the interface is not typehinted yet, but concrete classes are and contrib extending from them would fail.

Yes this would be a BC break. We have 2 contrib database drivers (MySQLi and MS SQL Server). The first one is being integrated in Drupal core and the second one does not override the method. I do not know of any other contrib overrides for the method and http://grep.xnddx.ru is no longer working. I do not know of any other alternatives.
If there are any overrides it will be very few instances. For me it is an exceptable risk. I leave it up to committer to make up his/her own mind and make the final decession. Back to RTBC.

The last submitted patch, 7: 3259740-7--test-only.patch, failed testing. View results →

Committed to 11.x

Created and published a change record

Automatically closed - issue fixed for 2 weeks with no activity.