Opened 9 years ago
Closed 9 years ago
#33206 closed defect (bug) (fixed)
Ignore the sitecategories table during network update if global terms are not enabled
Reported by: | nofearinc | Owned by: | dd32 |
---|---|---|---|
Milestone: | 4.3 | Priority: | low |
Severity: | normal | Version: | 4.2 |
Component: | Upgrade/Install | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
While updating a multisite network, I saw that in the error log:
WordPress database error Table 'automobile.au_sitecategories' doesn't exist for query SHOW FULL COLUMNS FROM `au_sitecategories` made by wp_upgrade, upgrade_network, maybe_convert_table_to_utf8mb4
I found that upgrade_network
runs maybe_convert_table_to_utf8mb4
in wp-admin/includes/upgrade.php
by fetching $wpdb->tables( 'global' );
which always returns the sitecategories table even if it wasn't defined.
I've added the solution provided by nacin for #12964 in both changeset upgrade statements to ignore the database query if global terms are not enabled. I also wonder whether this shouldn't be handled in the wpdb
class when calling the switch within the tables
function.
Attachments (6)
Change History (28)
#3
@
9 years ago
This really feels like it should be handled inside wpdb::tables()
, but there's no need to check the DB IMHO, just seeing if the feature is enabled or not is probably enough.
This ticket was mentioned in Slack in #core by obenland. View the logs.
9 years ago
#5
@
9 years ago
- Keywords needs-refresh added; commit removed
@nofearinc, could you update your patch with Dion's feedback?
#7
@
9 years ago
- Keywords needs-refresh removed
33206.diff: Am I doing this right? Should it only be unset if blog prefixes are requested?
#8
follow-up:
↓ 9
@
9 years ago
33206.diff causes a fatal error:
Fatal error: Call to undefined function wp_cache_get() in wp-includes/option.php on line 995
#9
in reply to:
↑ 8
;
follow-up:
↓ 13
@
9 years ago
Replying to SergeyBiryukov:
33206.diff causes a fatal error
In wp-settings.php, wp_set_wpdb_vars()
happens before wp_start_object_cache()
, so the object cache is not yet available. We're trying to call get_site_option()
via global_terms_enabled() too early.
Checking the DB table seems to work, see 33206.2.diff.
This ticket was mentioned in Slack in #core by obenland. View the logs.
9 years ago
#11
@
9 years ago
The disconnect between the setting and the existence of the table is slightly off-putting, but we'd have the same thing if we tried to check the setting, instead. With that in mind, I think checking for the table's existence is the more correct option for WPDB.
I don't think we have any global terms unit tests. It'd be nice to add a simple test for this, but it's probably the least of our concerns if we really wanted to test global terms.
This ticket was mentioned in Slack in #core by obenland. View the logs.
9 years ago
#13
in reply to:
↑ 9
@
9 years ago
Replying to SergeyBiryukov:
Replying to SergeyBiryukov:
33206.diff causes a fatal error
In wp-settings.php,
wp_set_wpdb_vars()
happens beforewp_start_object_cache()
, so the object cache is not yet available. We're trying to callget_site_option()
via global_terms_enabled() too early.
Checking the DB table seems to work, see 33206.2.diff.
We could flip it so that wp_set_wpdb_vars()
happens after wp_start_object_cache()
, however, get_site_option
still requires that wp_set_wpdb_vars()
has run, so we've got a circular dependency we can't work around.
So; I'd rather handle it outside of wpdb
if a query would always be required. 33206.3.diff is a untested version of nofearinc's patch which IMHO should be good to go.
#14
@
9 years ago
Just for fixing this specific bug, I'm cool with 33206.3.diff (minus the is_multisite()
check, which already happens in global_terms_enabled()
).
It kind of depends on if we want to put any effort at all into maintaining global terms, or if it's finally time to purge it.
#15
follow-up:
↓ 16
@
9 years ago
Upon further reflection, lets kill off global terms.
In the mean time, doing the check during upgrade is the least offensive method to fix this bug, though I prefer checking that the table exists, as we're performing operations directly on the table: 33206.4.diff.
#16
in reply to:
↑ 15
@
9 years ago
Replying to pento:
In the mean time, doing the check during upgrade is the least offensive method to fix this bug, though I prefer checking that the table exists, as we're performing operations directly on the table: 33206.4.diff.
Lets make that happen then! (Just noting, 33206.4.diff is very similar to 33206.patch)
#19
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
[33597] uses $this
when it should use $wpdb
, and causes fatal errors during network updates.
@pento/@dd32 could you take a look at this and punt/commit?