Ignore the sitecategories table during network update if global terms are not enabled

[nofearinc]

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.

[obenland]

@pento/@dd32 could you take a look at this and punt/commit?

[dd32]

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.

[obenland]

@nofearinc, could you update your patch with Dion's feedback?

[obenland]

33206.diff​: Am I doing this right? Should it only be unset if blog prefixes are requested?

[SergeyBiryukov]

33206.diff​ causes a fatal error:

Fatal error: Call to undefined function wp_cache_get() in wp-includes/option.php on line 995

[SergeyBiryukov]

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​.

[pento]

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.

[dd32]

untested; based off of nofearinc's patch

[dd32]

Replying to SergeyBiryukov:

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​.

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.

[pento]

Just for fixing this specific bug, I'm cool with 33206.3.diff​ for fixing this specific bug (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.

[pento]

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​.

[dd32]

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​)

[dd32]

In 33597:

Upgrade: Skip the sitecategories table when it doesn't exist (Global Terms is disabled).
Props nofearinc, obenland, SergeyBiryukov, and pento.
Fixes #33206

[boonebgorges]

[33597] uses $this when it should use $wpdb, and causes fatal errors during network updates.

[tellyworth]

replace $this with $wpdb in r33597

[azaozz]

33206.wpdb.diff fixes the typos, +1 to commit.

[azaozz]

In 33609:

Fix paste typos in upgrade.php.
Props tellyworth. Fixes #33206.