[wp-trac] [WordPress Trac] #38936: Alter Table Always Expects a COLUMN; Crashes on a CONSTRAINT

[WordPress Trac]

#38936: Alter Table Always Expects a COLUMN; Crashes on a CONSTRAINT
----------------------------------------+------------------------------
 Reporter:  philsown                    |       Owner:  (none)
     Type:  defect (bug)                |      Status:  new
 Priority:  normal                      |   Milestone:  Awaiting Review
Component:  Upgrade/Install             |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  needs-testing dev-feedback  |     Focuses:
----------------------------------------+------------------------------

Comment (by craigfrancis):

 Quick first thoughts (not run any code to check, and probably can't until
 later next week)...

 WordPress doesn't currently support Constraints, and doing so might get a
 bit complicated (especially when it comes to existing table records that
 don't conform to the constraint).

 ---

 upgrade.php currently does a fairly basic parse of the `CREATE TABLE`
 queries (aka `$cqueries`).

 It will run `DESCRIBE {$table}` to determine if the table currently
 exists. If that fails, it will simply run that SQL... if it succeeds, the
 table will be checked, and the table will be `ALTER`ed as necessary
 (adding fields/indexes).

 When doing the `ALTER` approach, it simply splits the query by line
 (`$flds`).

 [https://github.com/WordPress/wordpress-
 develop/blob/98fd38f44f0ef30fd136ec24aef6775f0503e3ff/src/wp-
 admin/includes/upgrade.php#L2817 /src/wp-admin/includes/upgrade.php#L2817]

 It tries to determine the `$fieldname` (the first word)... and if it
 matches 'primary', 'index', 'fulltext', 'unique', 'key', or 'spatial'; it
 is no longer considered a `$validfield` (good, because it's not a field),
 and will not be added to the `$cfields` array.

 Your SQL starts the line with `CONSTRAINT`, so it's incorrectly added to
 `$cfields`, and uses the `ADD COLUMN` approach (the same would happen if
 you had a line that started with `FOREIGN KEY`).

 So a new `case 'constraint':` needs to be added, and probably a few other
 changes in the rest of that bit of code, where it might end up in
 `$indices`... but it won't use the format "index_type index_name (...)".

 ---

 So, do we want to go down the route of supporting Constraints?

 They are useful to check the data in the tables, but they can introduce
 some complexity.

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