Skip to content

Commit

Permalink
Allow ALTER TABLE to operate on columns without COLUMN keyword (#145)
Browse files Browse the repository at this point in the history
This PR adds support for the following valid ALTER TABLE syntax:
```sql
ALTER TABLE <table-name> ADD <column-name> <data-type>;
```
and
```sql
ALTER TABLE <table-name> DROP <column-name>;
```

Prior to this PR, these queries do not work and result in error
conditions.

For reference, here is the documented SQLite syntax for ALTER TABLE
queries:
https://www.sqlite.org/syntax/alter-table-stmt.html

It shows that the COLUMN keyword is optional, and I've successfully
tested syntax without the COLUMN keyword in both MySQL and SQLite.

Reported by @JanJakes
  • Loading branch information
brandonpayton authored Aug 7, 2024
1 parent ac23456 commit 2561000
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 4 deletions.
86 changes: 85 additions & 1 deletion tests/WP_SQLite_Translator_Tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ enum_column ENUM('a', 'b', 'c') NOT NULL DEFAULT 'a',
);
}

public function testAlterTableAddColumn() {
public function testAlterTableAddAndDropColumn() {
$result = $this->assertQuery(
"CREATE TABLE _tmp_table (
name varchar(20) NOT NULL default ''
Expand Down Expand Up @@ -905,6 +905,90 @@ public function testAlterTableAddColumn() {
),
$results
);

$result = $this->assertQuery( 'ALTER TABLE _tmp_table ADD `column2` int;' );
$this->assertEquals( '', $this->engine->get_error_message() );
$this->assertEquals( 1, $result );

$this->assertQuery( 'DESCRIBE _tmp_table;' );
$results = $this->engine->get_query_results();
$this->assertEquals(
array(
(object) array(
'Field' => 'name',
'Type' => 'varchar(20)',
'Null' => 'NO',
'Key' => '',
'Default' => '',
'Extra' => '',
),
(object) array(
'Field' => 'column',
'Type' => 'int',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
(object) array(
'Field' => 'column2',
'Type' => 'int',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
),
$results
);

$result = $this->assertQuery( 'ALTER TABLE _tmp_table DROP COLUMN `column`;' );
$this->assertEquals( '', $this->engine->get_error_message() );
$this->assertEquals( 1, $result );

$this->assertQuery( 'DESCRIBE _tmp_table;' );
$results = $this->engine->get_query_results();
$this->assertEquals(
array(
(object) array(
'Field' => 'name',
'Type' => 'varchar(20)',
'Null' => 'NO',
'Key' => '',
'Default' => '',
'Extra' => '',
),
(object) array(
'Field' => 'column2',
'Type' => 'int',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
),
$results
);

$result = $this->assertQuery( 'ALTER TABLE _tmp_table DROP `column2`;' );
$this->assertEquals( '', $this->engine->get_error_message() );
$this->assertEquals( 1, $result );

$this->assertQuery( 'DESCRIBE _tmp_table;' );
$results = $this->engine->get_query_results();
$this->assertEquals(
array(
(object) array(
'Field' => 'name',
'Type' => 'varchar(20)',
'Null' => 'NO',
'Key' => '',
'Default' => '',
'Extra' => '',
),
),
$results
);
}

public function testAlterTableAddNotNullVarcharColumn() {
Expand Down
10 changes: 7 additions & 3 deletions wp-includes/sqlite/class-wp-sqlite-translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -2921,8 +2921,12 @@ private function execute_alter() {
$mysql_index_type = $this->normalize_mysql_index_type( $op_subject );
$is_index_op = (bool) $mysql_index_type;

if ( 'ADD' === $op_type && 'COLUMN' === $op_subject ) {
$column_name = $this->rewriter->consume()->value;
if ( 'ADD' === $op_type && ! $is_index_op ) {
if ( 'COLUMN' === $op_subject ) {
$column_name = $this->rewriter->consume()->value;
} else {
$column_name = $op_subject;
}

$skip_mysql_data_type_parts = $this->skip_mysql_data_type();
$sqlite_data_type = $skip_mysql_data_type_parts[0];
Expand All @@ -2940,7 +2944,7 @@ private function execute_alter() {
$column_name,
$mysql_data_type
);
} elseif ( 'DROP' === $op_type && 'COLUMN' === $op_subject ) {
} elseif ( 'DROP' === $op_type && ! $is_index_op ) {
$this->rewriter->consume_all();
} elseif ( 'CHANGE' === $op_type ) {
// Parse the new column definition.
Expand Down

0 comments on commit 2561000

Please sign in to comment.