[wp-trac] [WordPress Trac] #57385: Disable foreign key checks when dropping tables inside wp_uninitialize_site() function

[WordPress Trac]

#57385: Disable foreign key checks when dropping tables inside
wp_uninitialize_site() function
-------------------------------------------------+-------------------------
 Reporter:  naveen17797                          |       Owner:
                                                 |  johnjamesjacoby
     Type:  defect (bug)                         |      Status:  accepted
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Database                             |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  dev-feedback has-patch changes-      |     Focuses:  multisite
  requested                                      |
-------------------------------------------------+-------------------------
Changes (by johnjamesjacoby):

 * keywords:  dev-feedback has-patch => dev-feedback has-patch changes-
     requested
 * owner:  (none) => johnjamesjacoby
 * status:  new => accepted


Comment:

 Hey @naveen17797 👋 and thank you for this pull request.

 First, I like it 👍 and support merging it (or something like it) for a
 reason I'll tack onto the end.

 It is a small code change that will improve the chances that the
 ''un'initialization''/clean-up routine finishes completely, as intended by
 the documentation for the `wp_uninitialize_site()` function.

 Worth noting that WordPress core database tables do not implement `FOREIGN
 KEY` constraints (and have not since early-early-early days) which implies
 to me that this code change ''largely'' benefits a ''somewhat'' limited
 audience of:

 * modified core tables
 * plugins adding custom tables to the `blog` scope of `$wpdb->tables`

 At a cursory, only one other `DROP TABLE` usage appears to me like it
 would benefit from the same treatment, and that is in
 [https://core.trac.wordpress.org/browser/trunk/tests/phpunit/includes/utils.php#L471
 /tests/phpunit/includes/utils.php:Mock_Class::drop_tables()], which is a
 utility that is used with a similar intention but during unit testing
 instead of in production.

 (Other `DROP TABLE` usages are for specific ''core'' tables from previous
 WordPress versions that did not have foreign key constraints.)

 ----

 Off the top of my head, this change introduces a few possible
 consequential scenarios:

 * Anyone in the past 25 years who has been ''leaning'' on the fact that
 WordPress does not delete database tables with foreign key constraints
 would have a bad time. Some forensic analysis across the web would be
 necessary to determine if this approach has been published or popularized.
 * It adds to existing ''slippery slopes'' where code exists in WordPress
 to support things that WordPress itself does not use. I believe,
 personally, sometimes this makes sense and sometimes it doesn't, and our
 freedom to assess each decision independently has pros & cons (debate vs.
 paralysis).

 ----

 Regarding the code changes in the PR, before merging, I would:
 * make `foreign_key_checks`  uppercase
 * run the whitespace fixer
 * add some inline documentation before toggling the `SET` each time, to
 add helpful context to future contributors who may not immediately
 understand why that toggle is necessary
 * instead of hard-coding it to `1` and then `0`, it would be more
 developer & globally environmentally friendly to not assume that `0` is
 the default, and instead do a query to `GET` the current value and only
 toggle it if necessary.

 ----

 The main reason(s) I am +1 to add support for this, is because
 `wp_uninitialize_site()` specifically was coded with the intention of
 obliterating all site data, and subsequently similar considerations have
 been made to accommodate deleting all uploads – even in directories that
 are not limited to WordPress core ones.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57385#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform