WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 6 weeks ago

#41039 closed defect (bug) (fixed)

`_delete_option_fresh_site()` continually queries the database

Reported by: dlh Owned by: dlh
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

_delete_option_fresh_site() passes the integer 0 to update_option(), but 0 will later be fetched from the database as the string '0'.

Each time _delete_option_fresh_site() is called, then, when update_option() checks whether the new value is the same as the current value, it will see the two values as different and attempt to perform the database update.

The attached patch would instead pass the string '0' to update_option().

Attachments (3)

41039.diff (369 bytes) - added by dlh 3 months ago.
41039.1.diff (2.2 KB) - added by westonruter 6 weeks ago.
41039.2.diff (2.2 KB) - added by dlh 6 weeks ago.

Download all attachments as: .zip

Change History (7)

@dlh
3 months ago

#1 @westonruter
6 weeks ago

  • Component changed from Upgrade/Install to Customize
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.9
  • Owner set to westonruter
  • Status changed from new to accepted

This makes sense to me. It's ironic that for the abundance of loose type checking == equality in WordPress, an instance where strict === equality does exist here in update_option() and it causes a slight performance problem when the publish_post, publish_page, wp_ajax_save-widget, wp_ajax_widgets-order, or customize_save_after actions are done.

@westonruter
6 weeks ago

#2 @westonruter
6 weeks ago

  • Keywords reporter-feedback added
  • Owner changed from westonruter to dlh
  • Status changed from accepted to assigned

@dlh I was trying to add a unit test for this in 41039.1.diff but in doing so, I couldn't actually reproduce the issue when your patch was applied or not.

@dlh
6 weeks ago

#3 @dlh
6 weeks ago

  • Keywords reporter-feedback removed

@westonruter I think the second assertion passes when the patch isn't applied because the integer 0 is still cached after do_action( 'customize_save_after' ) was called for the first assertion.

When update_option() runs as part of the second do_action( 'customize_save_after' ), get_option() retrieves the cached integer, and the === test in update_option() passes.

41039.2.diff updates the test to regenerate the alloptions cache between assertions, which causes the second assertion to fail without the change to _delete_option_fresh_site().

#4 @westonruter
6 weeks ago

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

In 41244:

Customize: Prevent _delete_option_fresh_site() from hitting DB if fresh_site flag already cleared.

Amends [38991].
Props dlh, westonruter.
Fixes #41039.

Note: See TracTickets for help on using tickets.