It appears that either a typo or an oversight led to the php coding violation found on line 287 of the Database Core code in StatementPrefetch.php.

This code appears to only effect the SQLite database driver.

here's an attempt to fix the error by changing array_unshift into array_shift. Given the fact that only one argument is provided, and I can't think of what the second argument should be, that's what I"m going with.

Experts: please advise on what the proper fix should be.

ALSO, found another error on line 311 where case PDO::FETCH_COLUMN: used a mutli-dimension array for currentRow. I'm not sure if this is the proper fix but the $k variable it was using was undefined so I got rid of it.

Let's see what the testbots say:

CommentFileSizeAuthor
statmentprefetch.patch1.15 KBcosmicdreams

Comments

xjm’s picture

xjm
she/her
Primary language English
commented

Thanks for looking into this. However, I am unsure why this patch is needed. If it were a syntax error, there would be a syntax error. The code would not compile.

cosmicdreams’s picture

cosmicdreams commented

xjm: are you saying that the code is correct without the patch? or that there is something else going on?

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Closed (works as designed)

The former. I do not think the patch is needed. It looks like valid PHP to me. And if it weren't, it would blow up whenever the file is loaded, and drush si installs using SQLite so it seems very unlikely no one has ever loaded this file. :)

Marking "works as designed" unless we can get a clear indication that there is a problem with this code.

cosmicdreams’s picture

cosmicdreams commented
Status: Closed (works as designed) » Needs review

Marking "works as designed" unless we can get a clear indication that there is a problem with this code.

as you can see from php's documentation here: http://www.php.net/manual/en/function.array-unshift.php the second parameter is not optional. Does that convince you of the syntactical incorrectness of this line of code?

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs tests

Alright, if this is invalid PHP, we need an automated test covering this code, because it's clearly not getting run on the bot. Thanks @cosmicdreams!

We should be seeing:
Warning: Wrong parameter count for array_unshift() in /.../... on line N

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue summary: View changes 
+++ b/core/lib/Drupal/Core/Database/StatementPrefetch.php
@@ -284,7 +284,7 @@ class StatementPrefetch implements Iterator, StatementInterface {
-          $class_name = array_unshift($this->currentRow);
+          $class_name = array_shift($this->currentRow);

This part of the patch duplicates #1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Closed (duplicate)

Marking this as a duplicate of #1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly. This part of the patch:

+++ b/core/lib/Drupal/Core/Database/StatementPrefetch.php
@@ -308,7 +308,7 @@ class StatementPrefetch implements Iterator, StatementInterface {
           if (isset($this->columnNames[$this->fetchOptions['column']])) {
-            return $this->currentRow[$k][$this->columnNames[$this->fetchOptions['column']]];
+            return $this->currentRow[$this->columnNames[$this->fetchOptions['column']]];

Needs to be ported over to that issue's patch.