WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 4 months ago

#18609 new defect (bug)

term_id collisions possible with InnoDB tables and global_terms_enabled

Reported by: andrew_l Owned by:
Milestone: Future Release Priority: lowest
Severity: major Version: 3.2.1
Component: Taxonomy Keywords: needs-patch
Focuses: Cc:

Description

This bug relates to the "global terms" feature that uses the wp_sitecategories table to store all terms used across the multi-site install. This feature is enabled by setting global_terms_enabled to "1" in the wp_sitemeta table.

I have found that when global_terms_enabled is on, and you are using InnoDB tables, it is possible for a new term insertion in the local terms table (wp_x_terms) to fail due to an ID collision. This happens because InnoDB does not handle auto_increment in the same way as MyISAM, and the code for the global terms feature does not account for this difference.

In MyISAM, any change to a value in an auto-incremented column causes the table's AUTO_INCREMENT value (next value to be used, as seen via "show table status") to change. For example, if I have a MyISAM table, and I use an update statement to change the value of an auto incremented column to a value larger than the current AUTO_INCREMENT, the AUTO_INCREMENT is automatically increased as needed, so the next insertion cannot collide.

However, InnoDB does not do this. It only changes AUTO_INCREMENT on an insert. It does not change it when a column value is increased via an update statement. You can see this in the MySQL docs, or you can see it by creating a test case, like this:

  • Create an InnoDB table with 2 columns: ID (auto_increment, primary key) and data.
  • Insert a row using an insert statement, without specifying an ID.
  • Observe that ID = 1 for that row, as expected.
  • Use an update statement to set ID = 2 on that row.
  • Insert another row without specifying an ID.
  • Observe that the insertion fails. MySQL has attempted to insert a row with ID = 2, but there is already such a row.

If you do the same steps with a MyISAM table, the insertion does not fail, because AUTO_INCREMENT was automatically changed from 2 to 3 when your update statement was executed.

Going back into the context of the global terms feature: This problem happens when a new term is created in a site. The new term is initially inserted into the local terms table for the current site (wp_x_terms). After this, the function global_terms (in ms-functions.php) is called. This function finds or creates a matching entry in the wp_sitecategories global terms table, and uses an update statement to change the term_id of the row we just inserted in wp_x_terms. When wp_x_terms is an InnoDB table, the AUTO_INCREMENT value for the table does not change.

Suppose the AUTO_INCREMENT value was 58 after the original insertion, and further suppose that 58 was (coincidentally) the ID found in wp_sitecategories and written into our new row by the update statement described above. Now suppose we are ready to insert another new term into wp_x_terms. AUTO_INCREMENT is still 58, so we attempt to insert a term with term_id = 58, but it fails because there is already a row in the table with that ID.

Fix for this would be to reorder the operations so that no row is inserted into wp_x_terms until after wp_sitecategories has been queried and updated. That way, we know the ID we need to use when inserting into wp_x_terms before that insertion takes place, so we can specify it in the insert statement rather than depending on AUTO_INCREMENT.

I have marked this as severity major, because these unpredictable term_id collisions can cause weird errors, and the only workaround I have at this time is to disable global terms and restart MySQL (to regenerate AUTO_INCREMENT values on all tables).

Change History (5)

comment:1 unsalkorkmaz2 years ago

  • Cc unsalkorkmaz added

comment:2 wonderboymusic8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.7

Oh man - this is pretty dark, just confirmed it. Either this never happens and the ticket can be closed or IDs get changed and can blow up the auto_increment in InnoDB

comment:3 pento7 months ago

That's a pretty impressive find. It's documented InnoDB behaviour, so I wouldn't expect it to be fixed in MySQL ever.

Given how old and buggy the global terms code is (I spotted a couple of bugs in global_terms() just reading it), I'd be inclined to delete all of this code, and pretend it never happened.

comment:4 wonderboymusic7 months ago

  • Milestone changed from 3.7 to 3.8

No patch

comment:5 nacin4 months ago

  • Milestone changed from 3.8 to Future Release
  • Priority changed from normal to lowest

Would be really cool to fix, I guess. If WP.com still uses global terms, they may be the only ones.

Note: See TracTickets for help on using tickets.