Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#58423 closed defect (bug) (fixed)

Avoid an extra database query in populate_network()

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch good-first-bug needs-testing
Focuses: multisite, performance Cc:

Description

There is a $site_user value set in populate_network():

$site_user = get_userdata(
	(int) $wpdb->get_var(
		$wpdb->prepare(
			"SELECT meta_value
			FROM $wpdb->sitemeta
			WHERE meta_key = %s AND site_id = %d",
			'admin_user_id',
			$network_id
		)
	)
);

However, it is not actually used until these lines further in the function, inside an if ( ! is_multisite() ) check:

update_user_meta( $site_user->ID, 'source_domain', $domain );
update_user_meta( $site_user->ID, 'primary_blog', $current_site->blog_id );

To avoid an extra database query, the get_userdata() call can be moved directly above these two lines.

Follow-up to [12756], [35575], [43628].

Attachments (2)

58423.diff (1.2 KB) - added by nihar007 11 months ago.
58423-2.diff (1.7 KB) - added by nihar007 11 months ago.
I made one more changes for this ticket. Removed extra get_userdata function call & moved the query inside is_multisite condition block

Download all attachments as: .zip

Change History (11)

@nihar007
11 months ago

#1 @nihar007
11 months ago

  • Keywords needs-patch good-first-bug removed

Hi @SergeyBiryukov,
I removed the extra query from the place you suggested and I think this is valid.

This ticket was mentioned in PR #4523 on WordPress/wordpress-develop by @sakibmd.


11 months ago
#2

  • Keywords has-patch added

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


11 months ago

#4 @mukesh27
11 months ago

  • Keywords good-first-bug needs-testing added

#5 @mukesh27
11 months ago

This ticket was discussed in the recent bug scrub.

The ticket have two different approach that need to test. @SergeyBiryukov can you take a look it. Thanks.

Props to @costdev

@nihar007
11 months ago

I made one more changes for this ticket. Removed extra get_userdata function call & moved the query inside is_multisite condition block

#6 @sakibmd
11 months ago

I've created a PR about this issue @SergeyBiryukov. I think this solution may avoid the usage of extra database queries.

PR: https://github.com/WordPress/wordpress-develop/pull/4523

#7 @SergeyBiryukov
11 months ago

Thanks for the patches!

Good catch on removing the get_userdata() call in 58423-2.diff. I didn't notice it before, but I agree it's not really needed if we only use the ID.

#8 @SergeyBiryukov
11 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 55869:

Upgrade/Install: Avoid an extra database query in populate_network().

This moves the query for user ID of the new network's administrator closer to where the value is actually used.

Includes removing unnecessary get_userdata() call, as user ID is the only data needed here.

Follow-up to [12756], [35575], [43628].

Props nihar007, sakibmd, mukesh27, costdev, SergeyBiryukov.
Fixes #58423.

@SergeyBiryukov commented on PR #4523:


11 months ago
#9

Thanks for the PR! Merged in r55869.

Note: See TracTickets for help on using tickets.