Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#45582 closed defect (bug) (fixed)

PHP Notice in WP Site Query

Reported by: spacedmonkey's profile spacedmonkey Owned by: desrosj's profile desrosj
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.6
Component: Networks and Sites Keywords: has-patch php73
Focuses: multisite Cc:

Description

While running multisite in PHP 7.3, I get the following notice error.

`Notice: compact(): Undefined variable: groupby in /var/www/html/wordpress/wp-includes/class-wp-site-query.php on line 597`

This only appears on uncached site query calls, so it harder to notice.

Attachments (3)

45582.diff (396 bytes) - added by spacedmonkey 6 years ago.
45582.2.diff (644 bytes) - added by spacedmonkey 6 years ago.
45582.3.diff (721 bytes) - added by desrosj 6 years ago.
Move $groupby higher up to the correct location.

Download all attachments as: .zip

Change History (14)

@spacedmonkey
6 years ago

#1 @johnjamesjacoby
6 years ago

Confirmed. PHP 7.3 complains about this now.

@spacedmonkey
6 years ago

#2 @spacedmonkey
6 years ago

Updated the patch with another edge case.

#3 @pento
6 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#4 @desrosj
6 years ago

  • Owner set to desrosj
  • Status changed from new to assigned

These were fixed in [43832] and merged into trunk in [44166]. There is one issue that needs to be fixed in get_site_ids().

$groupby is initialized too late and can incorrectly overwrite values. This was caused in [44166].

@desrosj
6 years ago

Move $groupby higher up to the correct location.

#5 @spacedmonkey
6 years ago

@desrosj 45582.3.diff changes seem fine to me.

#6 @desrosj
6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 44186:

Networks and Sites: Fix incorrect variable location.

This fixes an issue introduced in [44166] where the $groupby variable was inserted too low in the get_site_ids() function while merging [43832] into trunk. The merged location did not account for a new conditional statement that existed only in trunk, and would have resulted in values assigned to $groupby being erased in certain scenarios.

Props spacedmonkey.

See #44416.
Fixes #45582.

#7 @spacedmonkey
6 years ago

@desrosj This change needs to be backdated all the way to 4.6, if possible.

#8 @desrosj
6 years ago

  • Milestone changed from 5.0.2 to 5.1

#9 follow-up: @spacedmonkey
6 years ago

@desrosj Why was this bumped to 5.1? This is a bug.

#10 in reply to: ↑ 9 @SergeyBiryukov
6 years ago

Replying to spacedmonkey:

Why was this bumped to 5.1? This is a bug.

The issue only exists in trunk due to the code added in [43010], where $groupby is defined inside a conditional block. The 5.0 branch does not have that code and should not have the issue.

#11 @desrosj
6 years ago

@SergeyBiryukov is correct. I should have closed this ticket as a duplicate of #44416 and fixed the issue there. New Sites API code only existed in trunk and the 5.0 branch code did not merge properly. [44186] will be a 5.1 change only.

Note: See TracTickets for help on using tickets.