Make WordPress Core

Opened 2 months ago

Last modified 8 weeks ago

#62997 accepted enhancement

Put the final nail in the coffin of shared terms

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile johnbillion
Milestone: 6.9 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch has-unit-tests
Focuses: performance Cc:

Description

Prior to WordPress 4.3, a term with a name that existed as both a category and a tag was actually a shared term. Shared terms were phased out in #21950 (4.1), #5809 (4.2), and #30261 (4.3). A site that's running 4.3 or later does not contain shared terms.

One item on the taxonomy roadmap from 2013 that was never completed was #30262 - combining the terms and term_taxonomy tables to facilitate the simplification of term queries.

Implementing this appears to be doable. I have a work in progress PR which is looking promising, however it is blocked by the fact that WordPress still supports shared terms. If a site upgrades from a version prior to 4.3 straight to the latest version then it's not guaranteed that the term_taxonomy table contains unique term_id values, which is a prerequisite for combining the terms and term_taxonomy tables.

To unblock this we need to ensure no shared terms are present. The upgrade routine should prevent the upgrade from continuing if shared terms might be present, and show a message explaining that the site must be updated to an interim version in order to first ensure any shared terms are split. This will only affect WordPress sites running a version prior to 4.3 and attempting to update to the latest version.

With this protection in place, all shared term handling can be removed and #30262 can proceed.

Change History (9)

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


2 months ago
#1

  • Keywords has-patch has-unit-tests added

#2 @johnbillion
2 months ago

  • Owner set to johnbillion
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core-performance by johnbillion. View the logs.


2 months ago

#4 follow-up: @jorbin
2 months ago

I think there is a step that needs to take place first and that is for 4.3 and earlier to no longer be receiving security updates unless this can be done in a way that allows for the term splitting to be completed before the DB change. This will be a rare (first that I can think of off the top of my head) instance where a site will not be able to "just upgrade" from any version to the latest version and if we are going to do that, we should do all we can to ensure as few sites as possible are running these severely out of date versions.

Additionally, for the breaking change to go in I think we will need to make sure that the updates API never offers whatever version this is in to sites below 4.3.

Finally, for folks looking for more on the history of shared terms, this speech from @boonebgorges covers the history very well: https://wordpress.tv/2015/12/20/boone-gorges-shared-terms-of-endearment-an-annotated-history-of-the-wordpress-taxonomy-component/

#5 in reply to: ↑ 4 @johnbillion
2 months ago

Replying to jorbin:

I think there is a step that needs to take place first and that is for 4.3 and earlier to no longer be receiving security updates

I don't quite follow here. Can you clarify?

This will be a rare (first that I can think of off the top of my head) instance where a site will not be able to "just upgrade" from any version to the latest version

I agree that this would likely be the first explicit instance, however I'm yet to determine whether it's actually possible to update from 4.2 to 6.8 in one go due to minimum supported PHP version constraints. 4.2 only officially supports PHP up to 5.6, but 6.8 requires 7.2 or higher. So a user would need to first be running WordPress 4.2 on PHP 7.2 or higher for the update to be possible. Does it work? I've yet to get an environment running to try it, although the upgrade tests on GitHub Actions indicate that it does. I will look into it further.

Additionally, for the breaking change to go in I think we will need to make sure that the updates API never offers whatever version this is in to sites below 4.3.

Yeah I think I agree, although the changes I'm proposing to upgrade.php will prevent the upgrade from proceeding if a user does attempt it. But yes it would be complementary to also make that change.

#6 @johnbillion
2 months ago

  • Milestone changed from 6.9 to 6.8

Moving to 6.8. Let's see how this progresses prior to beta 1.

#7 @spacedmonkey
2 months ago

+1 to this change!

#8 @flixos90
8 weeks ago

  • Milestone changed from 6.8 to 6.9

As Beta 1 is tomorrow and the PR has no approvals yet, I think this needs to be punted. If you can get it done before the Beta deadline, feel free to move back, but it seems a bit too late at this point - unless we want to prioritize wrapping it up in the 6.8 cycle and make it a "task (blessed)".

#9 @johnbillion
8 weeks ago

I would like to get this into 6.8 but I also think it's unnecessarily close to the beta, and I would appreciate some more eyes on it. So yes let's bump to 6.9.

Note: See TracTickets for help on using tickets.