Make WordPress Core

Opened 6 years ago

Last modified 4 months ago

#10404 new enhancement

dbDelta creates duplicate indexes when index definition contains spaces

Reported by: Denis-de-Bernardy Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8.1
Component: Database Keywords: has-patch
Focuses: Cc:


I was adding a much needed index in wp_object_term_relationships, and testing revealed it was getting added multiple times.

This works as intended:

CREATE TABLE $wpdb->term_relationships (
 object_id bigint(20) unsigned NOT NULL default 0,
 term_taxonomy_id bigint(20) unsigned NOT NULL default 0,
 term_order int(11) NOT NULL default 0,
 PRIMARY KEY  (object_id,term_taxonomy_id),
 UNIQUE KEY reverse_pkey (term_taxonomy_id,object_id),
 KEY term_taxonomy_id (term_taxonomy_id)
) $charset_collate;");

This doesn't:

CREATE TABLE $wpdb->term_relationships (
 object_id bigint(20) unsigned NOT NULL default 0,
 term_taxonomy_id bigint(20) unsigned NOT NULL default 0,
 term_order int(11) NOT NULL default 0,
 PRIMARY KEY  (object_id,term_taxonomy_id),
 UNIQUE KEY reverse_pkey (term_taxonomy_id, object_id),
 KEY term_taxonomy_id (term_taxonomy_id)
) $charset_collate;");

the only difference between the two is a space in the reverse_pkey column list. we should remove spaces in there to avoid potential bugs.

Attachments (5)

dbDelta_match_extra_spacing.diff (2.1 KB) - added by jbutkus 3 years ago.
Ignore whitespace issues within dbDelta duplicates recognition
10404.patch_case_insensitive_key_matching (2.1 KB) - added by charlestonsw 21 months ago.
Update case insensitive compare on dbDelta plus minor phpDoc updates.
10404.patch_caseinsensitive_and_noindexname_compare (5.5 KB) - added by charlestonsw 21 months ago.
Case insensitive index AND simple index with NO NAME patch (see comments for details)
10404_make_key_case_insensitive_wp_4_1.diff (1.4 KB) - added by charlestonsw 9 months ago.
4.1 patch candidate for case sensitivity fix
10404_case_sensitivity_fix_CORRECT_4_1.diff (917 bytes) - added by charlestonsw 9 months ago.
Case insensitive KEY without syntax errors.

Download all attachments as: .zip

Change History (21)

comment:1 @nacin6 years ago

  • Keywords needs-testing added

Patch should fix it, but I can't seem to fully reproduce. Can you test and confirm?

comment:2 @nacin6 years ago

  • Keywords needs-testing removed
  • Milestone changed from 2.9 to Future Release
  • Type changed from defect (bug) to task (blessed)

Patch does nothing, I jumped the gun there. The big problem here is that dbDelta wants exacting whitespace standards. The two spaces after "PRIMARY" is documented in the codex, but it also expects exactly one space elsewhere, and as you can see here, exactly no spaces separating field names in an index.

I'm setting this to Future Release and a blessed task to see if someone will pick it up, document dbDelta, and make it less fussy over whitespace.

See also #11322. At the very least, $wp_queries should lead by example.

comment:3 @nacin5 years ago

  • Type changed from task (blessed) to enhancement

@jbutkus3 years ago

Ignore whitespace issues within dbDelta duplicates recognition

comment:4 @jbutkus3 years ago

  • Cc butkus.justas@… added
  • Keywords has-patch added

I get, that this ticket is rather old, but I had experienced the very same issues with WP 3.4.2.

My patch is created against trunk version (https://core.svn.wordpress.org/trunk).

In one place (matching columns) preg_match regexp is extended by adding '\s*' instead of plain ' ' (single space - 0x20).

For indexes - I provided a function, which attempts to normalize indexes, by unifying various edge cases, namely: remove multiple whitespaces, replace 'INDEX' for 'KEY' at the beginning of declaration, etc.

These are based on DB DDLs used by our plugin (All-in-One Event Calendar by Timely). If someone encounters another edge case, which is not covered by these modifications - I may pay time to fix for these as well.

comment:5 @charlestonsw3 years ago

WP3.5.1 running WPMU is having the same issue.

Here is the error after a recent plugin activation:

WordPress database error Too many keys specified; max 64 keys allowed for query ALTER TABLE wp_2_store_locator ADD INDEX (sl_store) made by require('wp-blog-header.php'), require_once('wp-load.php'), require_once('wp-config.php'), require_once('wp-settings.php'), include_once('/plugins/store-locator-le/store-locator-le.php'), wpCSL_plugin__slplus->__construct, call_user_func_array, SLPlus_Activate::update, SLPlus_Activate->install_main_table, SLPlus_Activate->dbupdater, dbDelta

Here is the dbDelta structure from my plugin:

        	$sql = "CREATE TABLE $table_name (
                	sl_id mediumint(8) unsigned NOT NULL auto_increment,
                	sl_store varchar(255) NULL,
                	sl_address varchar(255) NULL,
                	sl_address2 varchar(255) NULL,
                	sl_city varchar(255) NULL,
                	sl_state varchar(255) NULL,
                	sl_zip varchar(255) NULL,
                	sl_country varchar(255) NULL,
                	sl_latitude varchar(255) NULL,
                	sl_longitude varchar(255) NULL,
                	sl_tags mediumtext NULL,
                	sl_description text NULL,
                	sl_email varchar(255) NULL,
                	sl_url varchar(255) NULL,
                	sl_hours varchar(255) NULL,
                	sl_phone varchar(255) NULL,
                	sl_fax varchar(255) NULL,
                	sl_image varchar(255) NULL,
                	sl_private varchar(1) NULL,
                	sl_neat_title varchar(255) NULL,
                	sl_linked_postid int NULL,
                	sl_pages_url varchar(255) NULL,
                	sl_pages_on varchar(1) NULL,
                	sl_option_value longtext NULL,
                	sl_lastupdated  timestamp NOT NULL default current_timestamp,   		 
                	PRIMARY KEY  (sl_id),
                	INDEX (sl_store),
                	INDEX (sl_longitude),
                	INDEX (sl_latitude)

The table now has a number of duplicate indexes:




Am I using dbDelta incorrectly or is this still a known issue? Any workarounds?

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

comment:6 follow-up: @RonStrilaeff3 years ago

I also get that this is old but it apparently there is still a problem with dbDelta creating duplicate indexes. I am creating a table within the init action hook like this:

function sv_create_favorites_table(){
	global $wpdb;
	$table_name = $wpdb->prefix . "favorites"; 
	$sql = "CREATE TABLE IF NOT EXISTS $table_name (
		fav_id bigint(20) not null auto_increment,
		post_id bigint(20) not null,
		user_id bigint(20) not null,
		fav_date timestamp not null,
		PRIMARY KEY  (fav_id),
		KEY (post_id),
		KEY (user_id)
	require_once(ABSPATH . 'wp-admin/includes/upgrade.php');

add_action( 'init', 'sv_create_favorites_table');

... and the only way I could prevent it from creating post_id_2, 3, 4, (and user_id_2, 3, 4), etc was to add the "IF NOT EXISTS" clause to the CREATE TABLE statement (as in my snippet above).

Last edited 3 years ago by RonStrilaeff (previous) (diff)

comment:7 @SergeyBiryukov3 years ago

  • Component changed from General to Database

comment:8 in reply to: ↑ 6 @charlestonsw3 years ago

Replying to RonStrilaeff:

... and the only way I could prevent it from creating post_id_2, 3, 4, (and user_id_2, 3, 4), etc was to add the "IF NOT EXISTS" clause to the CREATE TABLE statement (as in my snippet above).

How does dbDelta handle the "IF NOT EXISTS" when the data structure changes, for example adding a column during a plugin update? Standard SQL would not add the column because the table exists.

That could be a bad side effect of the workaround.

comment:9 @sebet2 years ago

I've ran into the duplicate key name problem when adding an index to a table. After some debugging on the dbDelta function I've found why this was happening. This criteria:

if (!(($aindex = array_search($index_string, $indices)) === false)) {

will only match an existing index if the column name includes the index length.


The comparison for this index will not match and will cause a duplicate index because the length was not specified

KEY movie_type_idx (movie_type),

The comparison for this index will match and the index will not be duplicated because the length was included

KEY movie_type_idx (movie_type(255)),

comment:10 follow-up: @charlestonsw21 months ago

When using dbDelta you MUST use an uppercase KEY qualifier on indices. If you use lowercase the query string comparison will not match the database meta data which comes back with an uppercase "KEY". The loop that compares the index portion of the SQL statement is case sensitive.

The patch file above: 10404.patch_case_insensitive_key_matching has phpDoc updates, some comments, and most importantly turns the index comparison into a case insensitive operation.

The next issue I am investigating is the way the query string is built. In my specific instance I am using this to build a key:

create table x (
id mediumint(8) not null autoincrement,
KEY (id)

That will never match the index comparison check as the database meta data return info that dbDelta builds into the following string:

KEY id (id)

Even with the string insensitive search the resulting KEY (ID) does not match KEY ID (ID).

I can fix this by updating my create table command to match the meta data exactly by replaceing KEY (id) in my data definition with KEY id (id), but IMO dbDelta should be a little more forgiving. Otherwise there are a lot of plugins creating extra indices and thus server load on millions of WordPress installations out there. The "Too many keys specified; max 64 keys allowed" message is far too prevalent.

comment:11 in reply to: ↑ 10 @SergeyBiryukov21 months ago

Replying to charlestonsw:

The patch file above: 10404.patch_case_insensitive_key_matching has phpDoc updates, some comments, and most importantly turns the index comparison into a case insensitive operation.

Looks like 10404.patch_case_insensitive_key_matching only contains four changes to the inline docs and one unrelated change in wp-admin/includes/post.php. Could you upload the correct patch?

@charlestonsw21 months ago

Update case insensitive compare on dbDelta plus minor phpDoc updates.

comment:12 @charlestonsw21 months ago

Revised patch added.

There are some phpDoc updates, more comments, and only a couple of lines of code changed to call strtoupper() in the appropriate places.

Still coding and testing a patch to help convert simple index creation statements like "KEY (id)" to the meta data generated "KEY id (id)" pattern to eliminate the mis-match on simple keys.

@charlestonsw21 months ago

Case insensitive index AND simple index with NO NAME patch (see comments for details)

comment:13 @charlestonsw21 months ago

A new patch file "10404.patch_caseinsensitive_and_noindexname_compare​" has been uploaded to help reduce the number of duplicate indices that are created by plugins that do not use the WordPress index creation methodology. Someone should probably make some notes around dbDelta in the WordPress docs as well as the syntax is rather specific. What the patch has:

  • A good bit more phpDoc info.
  • A LOT more comments around each code block.
  • Case insensitive comparison of index declaration blocks by shifting both the incoming index creation string and the metadata build strings to uppercase.
  • If the original version of the index comparison fails to find an EXACT match, it then falls back to check for "simple index keys". These are keys WITHOUT an index name defined, such as KEY (id) to index the id field.

The last part, which is well commented in the code, looks at the metadata-derived strings and converts the string that has been built as KEY <idxname> (<fldname>) and drops the optional index name portion if the index name EXACTLY MATCHES the field name portion.

In other words it converts the metadata-derived KEY <idxname> (<fldname>) to KEY (<fldname>) if both idxname and fldname are identical.

Since many plugins seem to like to use the shorthand KEY (<fldname>) syntax for building keys, this may help reduce the "too many keys" errors and improve performance during record insert/update operations on the plugin-provided tables.

This does not fix ALL possible dbDelta "too many keys" errors but it should help reduce the most common errors.

For developers the best method for using dbDelta with regard to key building is:

  • Always use an uppercase KEY directive.
  • Always include an index name in lowercase.
  • Always include your field list in lowercase.
  • Do not add field lengths to the keys.
  • When creating compound keys always eliminate whitespace. The means no spaces before or after parens or around commas.

In other words:
KEY mycompoundkey (id,slid)

key ( id , slid )

Personally I find the later (spaces around parens) hard to do after training myself to start ADDING spaces around parens to meet the WordPress coding standards which dictate logic operations to be ( a !== b ) versus (a!==b). :/

HTH some other code geeks out there.

@charlestonsw9 months ago

4.1 patch candidate for case sensitivity fix

@charlestonsw9 months ago

Case insensitive KEY without syntax errors.

comment:15 @charlestonsw9 months ago

file: 10404_case_sensitivity_fix_CORRECT_4_1.diff

Added a patch based on 4.1 source to address the case sensitivity issue on the KEY and index names. This will prevent the following from creating duplicate indices with dbDelta():

CREATE ... key (A,A);

Tested in 4.1 via manual code test.

Do we need to add phpUnit testing?

comment:16 @charlestonsw4 months ago

An article I wrote tonight: http://www.storelocatorplus.com/wordpress-dbdelta-better-in-4-2-not-yet-perfect/ DBDelta Review in 4.2.2. This is after cleaning up several websites that had n+ indices on the same exact key phrase. The clean-up happened after tracing performance issues. Turns out having a few-dozen of the same index declaration on a table can make for some very slow add/update operations on those tables.

I saw some https://wordpress.slack.com/archives/core/p1414565793002046 threads on Slack last October about forcing dbDelta() to adhere to a "subset of clean MySQL". No problems here, but we have a notable issue, IMO, when forcing plugin and theme developers to use this subset of SQL degrades the performance of websites that employ otherwise-well-written plugins. Especially when the new docs that tell devs "the right way" were just updated 6 months ago. There are 22,000+ plugins that wrote their CREATE TABLE command years ago and I doubt they will ever read that doc page again.

If you scan the plugin (and theme) source files you will find numerous cases where these two things happen quite frequently:

1) PRIMARY KEY is NOT followed by double spaces.

2) KEY IS followed by more than ONE space.

Item #1 does not cause long term performance issues. However it does force a data structure I/O on every single theme/plugin update. I just fixed this double-space issue in my plugin. For clients that are hosting 250,000+ locations in my custom table that one "small thing" that was hidden from the user was causing a VERY LONG WAIT during a simple plugin update while MySQL recreate, unnecessarily, a primary key.

Item #2 is a bigger issue. This creates duplicate indices on a table. If a table has 5 indices defined and the user updates 10 times over a couple of years they have 50+ indices. It is only a matter of time before MySQL starts throwing out more problematic maximum index count errors and warnings. Upgrades take longer. Data I/O takes longer. All-in-all not good for the performance of the WordPress site.

The question is:

Do you force developers to adhere to very strict standards when using dbDelta()?

If so, http://codex.wordpress.org/Creating_Tables_with_Plugins#Creating_or_Updating_the_Table the online doc updates are a step in the right direction but they need to be a LOT MORE DETAILED.

Also, you need to consider that many users of WordPress plugins and themes will suffer the hidden consequences of poor performance as the "index cruft" builds up through subsequent updates.

Yes, core is clean, but WordPress on a install site is an all-inclusive system.

IMO, dbDelta() should do whatever it can, within reason, to address basic variances in SQL statement structures. At the very least addressing idiosyncrasies in the KEY command itself "double space here, single space there" should be dealt with to make it consistent. That would be far more disruptive to core, however.

I'm willing to write some "clean up inbound SQL for dbDelta" code and build on the recent phpUnit tests that was noted on another ticket to validate the changes, but I don't want to burn time on Core code that won't get used.

My plugin now adheres to all of the quirks of dbDelta, so this is not a "for my plugin" concern. It is a concern for all those sites that now have 64 indexes in place, are taking a performance hit every single day without anyone having a clue.

The next question is how flexible do you make this? What is the real performance hit? dbDelta is run how often v. index queries and updates? If there is a concern, wrap a "DBDELTA_STRICT_DEFINES" constant around code you don't want to add the overhead to. Combine that with the newly-introduced globals list and skip the extra "massaging of SQL" for those things as well. The performance hit could be minimal while making dbDelta more code friendly.

As an aside, there are a LOT of plugins that use whitespace formatting to make nice readable code, using extra spaces to line up the column names and index names, then the field declaration. That is what my code used to look like, an was far easier to read the data structure from. Clean readable code and the dbDelta functional requirements are in direct opposition.

Possibly big changes going into dbDelta()... scary stuff given the potential impact. Maybe starting with a grep of plugin & theme code on the repo and getting some idea of malformed SQL is warranted to determine just how far-reaching this issue is.

Core Team comments?

Note: See TracTickets for help on using tickets.