[wp-trac] [WordPress Trac] #20263: Backticks in dbDelta cause warning and actually causes a query to alter all columns and indexes to run even if none have changed

[WordPress Trac]

#20263: Backticks in dbDelta cause warning and actually causes a query to alter all
columns and indexes to run even if none have changed
--------------------------------------------+------------------------
 Reporter:  Driskell                        |       Owner:  ocean90
     Type:  defect (bug)                    |      Status:  reviewing
 Priority:  normal                          |   Milestone:  4.6
Component:  Database                        |     Version:  1.5
 Severity:  normal                          |  Resolution:
 Keywords:  has-patch early has-unit-tests  |     Focuses:
--------------------------------------------+------------------------
Changes (by ocean90):

 * status:  new => reviewing
 * owner:   => ocean90
 * milestone:  Future Release => 4.6
 * keywords:  has-patch early needs-unit-tests => has-patch early has-unit-
     tests
 * severity:  minor => normal


Old description:

> Backticks in a CREATE TABLE in a dbDelta call are causing the below
> warning, and running a query to alter all columns and indexes with
> backticks even if they haven't changed:
> Notice: Undefined offset: 1 in /home/backuptechnology/public_html/wp-
> admin/includes/upgrade.php on line 1544
>
> Seems this started a long long time ago after this was fixed:
> http://core.trac.wordpress.org/ticket/8014
>
> Seems there is a match on line 1587 of WordPress 3.3.1 wp-
> admin/includes/upgrade.php:
> {{{
>                // For every field in the table
>                 foreach ($tablefields as $tablefield) {
>                         // If the table field exists in the field
> array...
>                         if
> (array_key_exists(strtolower($tablefield->Field), $cfields)) {
>                                 // Get the field type from the query
>                                 preg_match("|".$tablefield->Field." ([^
> ]*( unsigned)?)|i", $cfields[strtolower($tablefield->Field)], $matches);
> }}}
>
> The preg_match bit is what fails to match because it is looking for
> "fieldname" instead of "`fieldname`".
> So this is where the warning occurs, and why it assumes the type of the
> field is wrong.
> This means the previous patch 3 years ago probably meant when you have
> backticks in your CREATE TABLE, when you run dbDelta it actually runs an
> ALTER TABLE for every single column in the table, and then an ALTER TABLE
> ADD INDEX / PRIMARY for every single KEY and PRIMARY (there's more than
> just the above code that needs adjusting), causing duplicate key errors,
> but ignored. Guess it kinda works though...
>
> Also seems during the DESCRIBE it doesn't use backticks so if someone has
> a table with a reserved name (stupid I know) it would break dbDelta
> completely, but that's me digressing I just think getting rid of the
> warnings for CREATE TABLES with backticks will be good.
>
> But explains why my plugin takes a few seconds to activate hahaha. :-)
>
> I'll provide a patch at some point this week or next unless someone has
> already done so.

New description:

 Backticks in a CREATE TABLE in a dbDelta call are causing the below
 warning, and running a query to alter all columns and indexes with
 backticks even if they haven't changed:
 Notice: Undefined offset: 1 in /home/backuptechnology/public_html/wp-
 admin/includes/upgrade.php on line 1544

 Seems this started a long long time ago after this was fixed: #8014

 Seems there is a match on line 1587 of WordPress 3.3.1 wp-
 admin/includes/upgrade.php:
 {{{
                // For every field in the table
                 foreach ($tablefields as $tablefield) {
                         // If the table field exists in the field array...
                         if
 (array_key_exists(strtolower($tablefield->Field), $cfields)) {
                                 // Get the field type from the query
                                 preg_match("|".$tablefield->Field." ([^
 ]*( unsigned)?)|i", $cfields[strtolower($tablefield->Field)], $matches);
 }}}

 The preg_match bit is what fails to match because it is looking for
 "fieldname" instead of "`fieldname`".
 So this is where the warning occurs, and why it assumes the type of the
 field is wrong.
 This means the previous patch 3 years ago probably meant when you have
 backticks in your CREATE TABLE, when you run dbDelta it actually runs an
 ALTER TABLE for every single column in the table, and then an ALTER TABLE
 ADD INDEX / PRIMARY for every single KEY and PRIMARY (there's more than
 just the above code that needs adjusting), causing duplicate key errors,
 but ignored. Guess it kinda works though...

 Also seems during the DESCRIBE it doesn't use backticks so if someone has
 a table with a reserved name (stupid I know) it would break dbDelta
 completely, but that's me digressing I just think getting rid of the
 warnings for CREATE TABLES with backticks will be good.

 But explains why my plugin takes a few seconds to activate hahaha. :-)

 I'll provide a patch at some point this week or next unless someone has
 already done so.

--

--
Ticket URL: <https://core.trac.wordpress.org/ticket/20263#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform