WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#34872 new defect (bug)

dbDelta Missing Index Name Creates Duplicate Indexes

Reported by: charlestonsw Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.1
Component: Database Keywords: has-patch has-unit-tests
Focuses: performance Cc:

Description

Reference ticket #10404.

This is to decompose the original ticket into components. May be fixed in 4.4. Needs testing.


Creates duplicate indexes:

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

Does not create duplicate indexes:

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

Attachments (1)

34872.patch (2.9 KB) - added by jdgrimes 4 years ago.
Fix with unit tests

Download all attachments as: .zip

Change History (4)

@jdgrimes
4 years ago

Fix with unit tests

#1 @jdgrimes
4 years ago

  • Keywords has-patch has-unit-tests added

I've uploaded 34872.patch which fixes the issue and includes unit tests. The unit tests can be easily expanded to cover the many other issues from #10404, but I thought we might want to go ahead and get this committed and then take those one at a time, to make sure that we don't break anything.

This ticket was mentioned in Slack in #core by jdgrimes. View the logs.


4 years ago

#3 @charlestonsw
3 years ago

As of 4.4.02 this is still an issue.

Correct Syntax

    /**
     * This is the approved WP 4.0 format.
     *
     * Double space after PRIMARY KEY
     *
     * Other indexes have KEY <keyname> (<key field>)
     *
     * Does NOT generate duplicate indexes.
     * Does NOT generate database errors (see test 004 below)
     */
    private function index_test_001() {
        global $wpdb;
        $table_name = $wpdb->prefix . 'dbdelta_test_001';
        $wpdb_collate = $wpdb->collate;
        $sql =
            "CREATE TABLE {$table_name} (
            id mediumint(8) unsigned NOT NULL auto_increment ,
            first varchar(255) NULL,
            PRIMARY KEY  (id),
            KEY first (first)
            )
            COLLATE {$wpdb_collate}";

        require_once(ABSPATH . 'wp-admin/includes/upgrade.php');
        dbDelta( $sql );
        dbDelta( $sql );
    }

Test That Fails

    /**
     * Skips index name on secondary index.
     *
     * AS OF WP 4.4.2 this produces TWO indexes named "first"
     */
    private function index_test_003() {
        global $wpdb;
        $table_name = $wpdb->prefix . 'dbdelta_test_003';
        $wpdb_collate = $wpdb->collate;
        $sql =
            "CREATE TABLE {$table_name} (
            id mediumint(8) unsigned NOT NULL auto_increment ,
            first varchar(255) NULL,
            PRIMARY KEY  (id),
            KEY (first)
            )
            COLLATE {$wpdb_collate}";

        require_once(ABSPATH . 'wp-admin/includes/upgrade.php');
        dbDelta( $sql );
        dbDelta( $sql );
    }

Test That Works (using INDEX keyword) but generates WPDB duplicate key notice

    /**
     * Uses INDEX instead of KEY on second index.
     *
     * AS OF WP 4.4.2 this generates and error in dbDelta
     * DOES NOT create a duplicate index
     * WordPress database error Duplicate key name 'first' for query ALTER TABLE wp_dbdelta_test_004 ADD INDEX first (first)
     */
    private function index_test_004() {
        global $wpdb;
        $table_name = $wpdb->prefix . 'dbdelta_test_004';
        $wpdb_collate = $wpdb->collate;
        $sql =
            "CREATE TABLE {$table_name} (
            id mediumint(8) unsigned NOT NULL auto_increment ,
            first varchar(255) NULL,
            PRIMARY KEY  (id),
            INDEX first (first)
            )
            COLLATE {$wpdb_collate}";

        require_once(ABSPATH . 'wp-admin/includes/upgrade.php');
        dbDelta( $sql );
        dbDelta( $sql );
    }

I have these tests (and others) in a private plugin I've created named "db-delta-test". I'll post on Github if anyone is interested. Could be useful for doing phpUnit testing if someone wants to help me convert it to a proper phpUnit format.

Note: See TracTickets for help on using tickets.