Compatibility checks for PHP 7.2 surfaced a bug in DatabaseStatementPrefetch::current() where array_unshift() is called instead of array_shift().

A fix was committed to 8.7.x and 8.8.x, but still needs to be ported to 7.x.

Original summary:

The "element to prepend" parameter for function | array_unshift() is missing, but was required for PHP | version 7.2 and lower

case PDO::FETCH_CLASS | PDO::FETCH_CLASSTYPE:
$class_name = array_shift($this->currentRow);

ref: http://www.php.net/manual/en/function.array-unshift.php

CommentFileSizeAuthor
prefetcharrayshift.patch669 byteskaran_mudi
#2 prefetcharrayshift-3064734-2.patch639 bytesgeoffreyr
#8 3064734-8-test-only.patch3.67 KBsjerdo
#8 3064734-8.patch4.3 KBsjerdo
#10 3064734-9-test-only.patch3.67 KBsjerdo
#10 3064734-9.patch4.29 KBsjerdo
#10 interdiff-3064734-8-9.patch4.29 KBsjerdo
#11 interdiff-3064734-8-9.patch529 bytessjerdo

Issue fork drupal-3064734

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

karan_mudi created an issue. See original summary.

geoffreyr’s picture

geoffreyr commented
Related issues: +#1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly
StatusFileSize
new prefetcharrayshift-3064734-2.patch639 bytes

Removed the docroot/ prefix, but looks sound and in line with other fixes such as this.

geoffreyr’s picture

geoffreyr commented
Status: Active » Needs review
sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
Title: [PHP 7.2] Fully support PHP 7.2 in Drupal 7 » [PHP 7.2] Fix Error 'The "element to prepend" parameter for function array_unshift() is missing, but was required'
Status: Needs review » Reviewed & tested by the community
Issue tags: +PHP 7.2, +Needs tests

Looks good to me, but I think we need a test here.

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
Status: Reviewed & tested by the community » Needs work
ciss’s picture

ciss commented
Title: [PHP 7.2] Fix Error 'The "element to prepend" parameter for function array_unshift() is missing, but was required' » DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly
Version: 7.67 » 7.x-dev
Assigned: karan_mudi » Unassigned
Category: Support request » Bug report
Priority: Normal » Major
Issue summary: View changes
Issue tags: -PHP 7.2
Parent issue: » #1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly
Related issues: -#1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly

I'd argue that this has nothing to do with PHP 7.2. The issue covers a bug that was only surfaced by the compatibility checks and should instead act as backport issue for #1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly. Setting to major to match parent priority.

iarc13’s picture

iarc13 commented

array_unshift() returns the new number of elements. So why not use count() instead if there are no new values being appended to it?

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
Status: Needs work » Needs review
StatusFileSize
new 3064734-8-test-only.patch3.67 KB
new 3064734-8.patch4.3 KB

Backport of test and patch in #1476782: DatabaseStatementPrefetch::current PHP function array_unshift() are used incorrectly

sjerdo opened merge request !788

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
StatusFileSize
new 3064734-9-test-only.patch3.67 KB
new 3064734-9.patch4.29 KB
new interdiff-3064734-8-9.patch4.29 KB

Fixed field type for job column in test schema

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
StatusFileSize
new interdiff-3064734-8-9.patch529 bytes

Fixed interdiff between patch 8 and 9

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
commented
poker10’s picture

poker10 commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +sqlite

Thanks for the patch. I have tested it and it is working as expected. It is a straight backport of the D8 issue and the test failures are present as in D8. This only affects SQLITE, see: https://www.drupal.org/comment/5740380. The patch also has tests, so I am setting this as RTBC.

mcdruid’s picture

mcdruid commented
Issue tags: +RTBM

https://git.drupalcode.org/project/drupal/-/blob/9.4.5/core/lib/Drupal/C...

Looks good, thanks!

  • poker10 committed 782debf on 7.x 
    Issue #3064734 by sjerdo, geoffreyr, karan_mudi:...
poker10’s picture

poker10 commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -RTBM

Thanks all!

Status: Fixed » Closed (fixed)

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

sjerdo closed merge request !788