WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#10404 new enhancement

dbDelta creates duplicate indexes when index definition contains spaces

Reported by: Denis-de-Bernardy Owned by:
Priority: normal Milestone: Future Release
Component: Database Version: 2.8.1
Severity: normal Keywords: has-patch
Cc: butkus.justas@…

Description

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 (1)

dbDelta_match_extra_spacing.diff (2.1 KB) - added by jbutkus 8 months ago.
Ignore whitespace issues within dbDelta duplicates recognition

Download all attachments as: .zip

Change History (10)

comment:1 nacin4 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 nacin4 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 nacin3 years ago

  • Type changed from task (blessed) to enhancement

jbutkus8 months ago

Ignore whitespace issues within dbDelta duplicates recognition

comment:4 jbutkus8 months 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 charlestonsw4 months 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)
                	)
                	$charset_collate
                	";
dbDelta($sql)

The table now has a number of duplicate indexes:

sl_store
sl_store_2
sl_store_3
...
sl_store_21

sl_latitude
sl_latitude_2
...
sl_latitude_21

sl_longitude
sl_longitude_2
...
sl_longitude_21

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

Last edited 4 months ago by SergeyBiryukov (previous) (diff)

comment:6 follow-up: RonStrilaeff4 months 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');
	dbDelta($sql);
};

add_action( 'init', 'sv_create_favorites_table');

... and the only way I could prevent it from creating post_id_2, 3, 4, etc was to add the "IF NOT EXISTS" clause to the CREATE TABLE statement.

Version 0, edited 4 months ago by RonStrilaeff (next)

comment:7 SergeyBiryukov4 months ago

  • Component changed from General to Database

comment:8 in reply to: ↑ 6 charlestonsw4 months 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 sebet3 months 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.

Example:

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)),
Note: See TracTickets for help on using tickets.