Changeset 37583

Timestamp:

05/27/2016 11:56:20 AM (8 years ago)

Author:

ocean90

Message:

Database: Normalize index definitions in dbDelta().

dbDelta() compares the index definitions against the result of SHOW INDEX FROM $table_name. This requires a specific format so indices are not unnecessarily re-created. This format wasn't ensured, until now.

• Parse the raw index definition to extract the type, name and columns so a normalized definition can be built (#20263, #34873).
• Standardize on uppercase types (#34871) and on 'KEY'. 'INDEX' is only a synonym for 'KEY'.
• Escape index names with backticks (#20263).
• Normalize columns: Ignore ASC and DESC definitions (#34959), remove whitespaces (#34869) and escape column names with backticks (#20263).
• Add backticks to all index change queries (#20263).

Props ocean90, pento, kurtpayne.
Fixes #20263, #34869, #34871, #34873, #34959.

Location:

trunk

Files:

• src/wp-admin/includes/upgrade.php (modified) (6 diffs)
• tests/phpunit/tests/dbdelta.php (modified) (1 diff)

trunk/src/wp-admin/includes/upgrade.php

@@ -2185 +2185 @@
 
         // For every field line specified in the query.
-        foreach ($flds as $fld) {
+        foreach ( $flds as $fld ) {
+            $fld = trim( $fld, " \t\n\r\0\x0B," ); // Default trim characters, plus ','.
 
             // Extract the field name.
-            preg_match("|^([^ ]*)|", trim($fld), $fvals);
+            preg_match( '|^([^ ]*)|', $fld, $fvals );
             $fieldname = trim( $fvals[1], '`' );
             $fieldname_lowercased = strtolower( $fieldname );
@@ -2203 +2204 @@
                 case 'spatial':
                     $validfield = false;
-                    $indices[] = trim(trim($fld), ", \n");
+
+                    /*
+                     * Normalize the index definition.
+                     *
+                     * This is done so the definition can be compared against the result of a
+                     * `SHOW INDEX FROM $table_name` query which returns the current table
+                     * index information.
+                     */
+
+                    // Extract type, name and columns from the definition.
+                    preg_match(
+                          '/^'
+                        .   '(?P<index_type>'             // 1) Type of the index.
+                        .       'PRIMARY\s+KEY|(?:UNIQUE|FULLTEXT|SPATIAL)\s+(?:KEY|INDEX)|KEY|INDEX'
+                        .   ')'
+                        .   '\s+'                         // Followed by at least one white space character.
+                        .   '(?:'                         // Name of the index. Optional if type is PRIMARY KEY.
+                        .       '`?'                      // Name can be escaped with a backtick.
+                        .           '(?P<index_name>'     // 2) Name of the index.
+                        .               '(?:[0-9a-zA-Z$_-]|[\xC2-\xDF][\x80-\xBF])+'
+                        .           ')'
+                        .       '`?'                      // Name can be escaped with a backtick.
+                        .       '\s+'                     // Followed by at least one white space character.
+                        .   ')*'
+                        .   '\('                          // Opening bracket for the columns.
+                        .       '(?P<index_columns>'
+                        .           '.+?'                 // 3) Column names, index prefixes, and orders.
+                        .       ')'
+                        .   '\)'                          // Closing bracket for the columns.
+                        . '$/im',
+                        $fld,
+                        $index_matches
+                    );
+
+                    // Uppercase the index type and normalize space characters.
+                    $index_type = strtoupper( preg_replace( '/\s+/', ' ', trim( $index_matches['index_type'] ) ) );
+
+                    // 'INDEX' is a synonym for 'KEY', standardize on 'KEY'.
+                    $index_type = str_replace( 'INDEX', 'KEY', $index_type );
+
+                    // Escape the index name with backticks. An index for a primary key has no name.
+                    $index_name = ( 'PRIMARY KEY' === $index_type ) ? '' : '`' . $index_matches['index_name'] . '`';
+
+                    // Parse the columns. Multiple columns are separated by a comma.
+                    $index_columns = array_map( 'trim', explode( ',', $index_matches['index_columns'] ) );
+
+                    // Normalize columns.
+                    foreach ( $index_columns as &$index_column ) {
+                        // Extract column name and number of indexed characters (sub_part).
+                        preg_match(
+                              '/'
+                            .   '`?'                      // Name can be escaped with a backtick.
+                            .       '(?P<column_name>'    // 1) Name of the column.
+                            .           '(?:[0-9a-zA-Z$_-]|[\xC2-\xDF][\x80-\xBF])+'
+                            .       ')'
+                            .   '`?'                      // Name can be escaped with a backtick.
+                            .   '(?:'                     // Optional sub part.
+                            .       '\s*'                 // Optional white space character between name and opening bracket.
+                            .       '\('                  // Opening bracket for the sub part.
+                            .           '\s*'             // Optional white space character after opening bracket.
+                            .           '(?P<sub_part>'
+                            .               '\d+'         // 2) Number of indexed characters.
+                            .           ')'
+                            .           '\s*'             // Optional white space character before closing bracket.
+                            .        '\)'                 // Closing bracket for the sub part.
+                            .   ')?'
+                            . '/',
+                            $index_column,
+                            $index_column_matches
+                        );
+
+                        // Escape the column name with backticks.
+                        $index_column = '`' . $index_column_matches['column_name'] . '`';
+
+                        // Append the optional sup part with the number of indexed characters.
+                        if ( isset( $index_column_matches['sub_part'] ) ) {
+                            $index_column .= '(' . $index_column_matches['sub_part'] . ')';
+                        }
+                    }
+
+                    // Build the normalized index definition and add it to the list of indices.
+                    $indices[] = "{$index_type} {$index_name} (" . implode( ',', $index_columns ) . ")";
+
+                    // Destroy no longer needed variables.
+                    unset( $index_column, $index_column_matches, $index_matches, $index_type, $index_name, $index_columns );
+
                     break;
             }
-            $fld = trim( $fld );
 
             // If it's a valid field, add it to the field array.
             if ( $validfield ) {
-                $cfields[ $fieldname_lowercased ] = trim( $fld, ", \n" );
+                $cfields[ $fieldname_lowercased ] = $fld;
             }
         }
@@ -2244 +2329 @@
                     if ( $do_change ) {
                         // Add a query to change the column type.
-                        $cqueries[] = "ALTER TABLE {$table} CHANGE COLUMN {$tablefield->Field} " . $cfields[ $tablefield_field_lowercased ];
+                        $cqueries[] = "ALTER TABLE {$table} CHANGE COLUMN `{$tablefield->Field}` " . $cfields[ $tablefield_field_lowercased ];
                         $for_update[$table.'.'.$tablefield->Field] = "Changed type of {$table}.{$tablefield->Field} from {$tablefield->Type} to {$fieldtype}";
                     }
@@ -2254 +2339 @@
                     if ($tablefield->Default != $default_value) {
                         // Add a query to change the column's default value
-                        $cqueries[] = "ALTER TABLE {$table} ALTER COLUMN {$tablefield->Field} SET DEFAULT '{$default_value}'";
+                        $cqueries[] = "ALTER TABLE {$table} ALTER COLUMN `{$tablefield->Field}` SET DEFAULT '{$default_value}'";
                         $for_update[$table.'.'.$tablefield->Field] = "Changed default value of {$table}.{$tablefield->Field} from {$tablefield->Default} to {$default_value}";
                     }
@@ -2307 +2392 @@
                 }
                 $index_string .= 'KEY ';
-                if ($index_name != 'PRIMARY') {
-                    $index_string .= $index_name;
+                if ( 'PRIMARY' !== $index_name  ) {
+                    $index_string .= '`' . $index_name . '`';
                 }
                 $index_columns = '';
@@ -2314 +2399 @@
                 // For each column in the index.
                 foreach ($index_data['columns'] as $column_data) {
-                    if ($index_columns != '') $index_columns .= ',';
+                    if ( $index_columns != '' ) {
+                        $index_columns .= ',';
+                    }
 
                     // Add the field to the column list string.
-                    $index_columns .= $column_data['fieldname'];
+                    $index_columns .= '`' . $column_data['fieldname'] . '`';
                     if ($column_data['subpart'] != '') {
                         $index_columns .= '('.$column_data['subpart'].')';

trunk/tests/phpunit/tests/dbdelta.php

@@ -532 +532 @@
         $this->assertSame( array(
             "{$wpdb->prefix}spatial_index_test.spatial_value2" => "Added column {$wpdb->prefix}spatial_index_test.spatial_value2",
-            "Added index {$wpdb->prefix}spatial_index_test SPATIAL KEY spatial_key2 (spatial_value2)"
+            "Added index {$wpdb->prefix}spatial_index_test SPATIAL KEY `spatial_key2` (`spatial_value2`)"
             ), $updates );
 
         $wpdb->query( "DROP TABLE IF EXISTS {$wpdb->prefix}spatial_index_test" );
     }
+
+    /**
+     * @ticket 20263
+     */
+    function test_query_with_backticks_does_not_cause_a_query_to_alter_all_columns_and_indices_to_run_even_if_none_have_changed() {
+        global $wpdb;
+
+        $schema = "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test2 (
+                `id` bigint(20) NOT NULL AUTO_INCREMENT,
+                `references` varchar(255) NOT NULL,
+                PRIMARY KEY  (`id`),
+                KEY `compound_key` (`id`,`references`)
+            )
+        ";
+
+        $wpdb->query( $schema );
+
+        $updates = dbDelta( $schema );
+
+        $table_indices = $wpdb->get_results( "SHOW INDEX FROM {$wpdb->prefix}dbdelta_test2" );
+        $compound_key_index = wp_list_filter( $table_indices, array( 'Key_name' => 'compound_key' ) );
+
+        $wpdb->query( "DROP TABLE IF EXISTS {$wpdb->prefix}dbdelta_test2" );
+
+        $this->assertCount( 2, $compound_key_index );
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 20263
+     */
+    function test_index_with_a_reserved_keyword_can_be_created() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                `references` varchar(255) NOT NULL,
+                PRIMARY KEY  (id),
+                KEY key_1 (column_1),
+                KEY compound_key (id , column_1),
+                KEY compound_key2 (id,`references`),
+                FULLTEXT KEY fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $table_indices = $wpdb->get_results( "SHOW INDEX FROM {$wpdb->prefix}dbdelta_test" );
+
+        $this->assertCount( 2, wp_list_filter( $table_indices, array( 'Key_name' => 'compound_key2' ) , 'AND' ) );
+
+        $this->assertSame(
+            array(
+                "{$wpdb->prefix}dbdelta_test.references" => "Added column {$wpdb->prefix}dbdelta_test.references",
+                0 => "Added index {$wpdb->prefix}dbdelta_test KEY `compound_key2` (`id`,`references`)",
+            ),
+            $updates
+        );
+    }
+
+    /**
+     * @ticket 20263
+     */
+    function test_wp_get_db_schema_does_no_alter_queries_on_existing_install() {
+        $updates = dbDelta( wp_get_db_schema() );
+
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 20263
+     */
+    function test_key_and_index_and_fulltext_key_and_fulltext_index_and_unique_key_and_unique_index_indicies() {
+        global $wpdb;
+
+        $schema = "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                KEY key_1 (column_1),
+                KEY compound_key (id,column_1),
+                FULLTEXT KEY fulltext_key (column_1),
+                INDEX key_2 (column_1),
+                UNIQUE KEY key_3 (column_1),
+                UNIQUE INDEX key_4 (column_1),
+                FULLTEXT INDEX key_5 (column_1),
+            ) ENGINE=MyISAM
+        ";
+
+        $creates = dbDelta( $schema );
+        $this->assertSame(
+            array(
+                0 => "Added index {$wpdb->prefix}dbdelta_test KEY `key_2` (`column_1`)",
+                1 => "Added index {$wpdb->prefix}dbdelta_test UNIQUE KEY `key_3` (`column_1`)",
+                2 => "Added index {$wpdb->prefix}dbdelta_test UNIQUE KEY `key_4` (`column_1`)",
+                3 => "Added index {$wpdb->prefix}dbdelta_test FULLTEXT KEY `key_5` (`column_1`)",
+            ),
+            $creates
+        );
+
+        $updates = dbDelta( $schema );
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 20263
+     */
+    function test_index_and_key_are_synonyms_and_do_not_recreate_indices() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                INDEX key_1 (column_1),
+                INDEX compound_key (id,column_1),
+                FULLTEXT INDEX fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 20263
+     */
+    function test_indices_with_prefix_limits_are_created_and_do_not_recreate_indices() {
+        global $wpdb;
+
+        $schema = "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                KEY key_1 (column_1),
+                KEY compound_key (id,column_1),
+                FULLTEXT KEY fulltext_key (column_1),
+                KEY key_2 (column_1(10)),
+                KEY key_3 (column_2(100),column_1(10)),
+            ) ENGINE=MyISAM
+        ";
+
+        $creates = dbDelta( $schema );
+        $this->assertSame(
+            array(
+                0 => "Added index {$wpdb->prefix}dbdelta_test KEY `key_2` (`column_1`(10))",
+                1 => "Added index {$wpdb->prefix}dbdelta_test KEY `key_3` (`column_2`(100),`column_1`(10))",
+            ),
+            $creates
+        );
+
+        $updates = dbDelta( $schema );
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 34959
+     */
+    function test_index_col_names_with_order_do_not_recreate_indices() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                KEY key_1 (column_1 DESC),
+                KEY compound_key (id,column_1 ASC),
+                FULLTEXT KEY fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 34873
+     */
+    function test_primary_key_with_single_space_does_not_recreate_index() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY (id),
+                KEY key_1 (column_1),
+                KEY compound_key (id,column_1),
+                FULLTEXT KEY fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 34869
+     */
+    function test_index_definitions_with_spaces_do_not_recreate_indices() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                KEY key_1        (         column_1),
+                KEY compound_key (id,      column_1),
+                FULLTEXT KEY fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $this->assertEmpty( $updates );
+    }
+
+    /**
+     * @ticket 34871
+     */
+    function test_index_types_are_not_case_sensitive_and_do_not_recreate_indices() {
+        global $wpdb;
+
+        $updates = dbDelta(
+            "
+            CREATE TABLE {$wpdb->prefix}dbdelta_test (
+                id bigint(20) NOT NULL AUTO_INCREMENT,
+                column_1 varchar(255) NOT NULL,
+                column_2 text,
+                column_3 blob,
+                PRIMARY KEY  (id),
+                key key_1 (column_1),
+                key compound_key (id,column_1),
+                FULLTEXT KEY fulltext_key (column_1)
+            ) ENGINE=MyISAM
+            "
+        );
+
+        $this->assertEmpty( $updates );
+    }
 }