Skip to content

Fix & improve ON UPDATE trigger behavior #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Fix & improve ON UPDATE trigger behavior #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4f2f43
Fix the ON UPDATE to work for tables without "id" column
JanJakes Aug 15, 2024
344196a
Apply ON UPDATE CURRENT TIMESTAMP in CHANGE queries & fix trigger del…
JanJakes Aug 16, 2024
e59eff8
Add missing "skip" call
JanJakes Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 125 additions & 2 deletions tests/WP_SQLite_Translator_Tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,14 +1102,14 @@ public function testColumnWithOnUpdate() {
'name' => '___tmp_table_created_at_on_update__',
'tbl_name' => '_tmp_table',
'rootpage' => '0',
'sql' => "CREATE TRIGGER \"___tmp_table_created_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"created_at\" = CURRENT_TIMESTAMP WHERE id = NEW.id;\n\t\t\tEND",
'sql' => "CREATE TRIGGER \"___tmp_table_created_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"created_at\" = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
),
(object) array(
'type' => 'trigger',
'name' => '___tmp_table_updated_at_on_update__',
'tbl_name' => '_tmp_table',
'rootpage' => '0',
'sql' => "CREATE TRIGGER \"___tmp_table_updated_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"updated_at\" = CURRENT_TIMESTAMP WHERE id = NEW.id;\n\t\t\tEND",
'sql' => "CREATE TRIGGER \"___tmp_table_updated_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"updated_at\" = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
),
),
$results
Expand Down Expand Up @@ -1176,6 +1176,129 @@ public function testColumnWithOnUpdate() {
$this->assertNull( $result[0]->updated_at );
}

public function testColumnWithOnUpdateAndNoIdField() {
// CREATE TABLE with ON UPDATE
$this->assertQuery(
'CREATE TABLE _tmp_table (
name varchar(20) NOT NULL,
created_at timestamp NULL ON UPDATE CURRENT_TIMESTAMP
);'
);

// on INSERT, no timestamps are expected
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('aaa')" );
$result = $this->assertQuery( "SELECT * FROM _tmp_table WHERE name = 'aaa'" );
$this->assertNull( $result[0]->created_at );

// on UPDATE, we expect timestamps in form YYYY-MM-DD HH:MM:SS
$this->assertQuery( "UPDATE _tmp_table SET name = 'bbb' WHERE name = 'aaa'" );
$result = $this->assertQuery( "SELECT * FROM _tmp_table WHERE name = 'bbb'" );
$this->assertRegExp( '/\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d/', $result[0]->created_at );
}

public function testChangeColumnWithOnUpdate() {
// CREATE TABLE with ON UPDATE
$this->assertQuery(
'CREATE TABLE _tmp_table (
id int(11) NOT NULL,
created_at timestamp NULL
);'
);
$results = $this->assertQuery( 'DESCRIBE _tmp_table;' );
$this->assertEquals(
array(
(object) array(
'Field' => 'id',
'Type' => 'int(11)',
'Null' => 'NO',
'Key' => '',
'Default' => '0',
'Extra' => '',
),
(object) array(
'Field' => 'created_at',
'Type' => 'timestamp',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
),
$results
);

// no ON UPDATE is set
$this->assertQuery( 'INSERT INTO _tmp_table (id) VALUES (1)' );
$this->assertQuery( 'UPDATE _tmp_table SET id = 1 WHERE id = 1' );
$result = $this->assertQuery( 'SELECT * FROM _tmp_table WHERE id = 1' );
$this->assertNull( $result[0]->created_at );

// CHANGE COLUMN to add ON UPDATE
$this->assertQuery(
'ALTER TABLE _tmp_table CHANGE COLUMN created_at created_at timestamp NULL ON UPDATE CURRENT_TIMESTAMP'
);
$results = $this->assertQuery( 'DESCRIBE _tmp_table;' );
$this->assertEquals(
array(
(object) array(
'Field' => 'id',
'Type' => 'int(11)',
'Null' => 'NO',
'Key' => '',
'Default' => '0',
'Extra' => '',
),
(object) array(
'Field' => 'created_at',
'Type' => 'timestamp',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
),
$results
);

// now, ON UPDATE SHOULD BE SET
$this->assertQuery( 'UPDATE _tmp_table SET id = 1 WHERE id = 1' );
$result = $this->assertQuery( 'SELECT * FROM _tmp_table WHERE id = 1' );
$this->assertRegExp( '/\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d/', $result[0]->created_at );

// change column to remove ON UPDATE
$this->assertQuery(
'ALTER TABLE _tmp_table CHANGE COLUMN created_at created_at timestamp NULL'
);
$results = $this->assertQuery( 'DESCRIBE _tmp_table;' );
$this->assertEquals(
array(
(object) array(
'Field' => 'id',
'Type' => 'int(11)',
'Null' => 'NO',
'Key' => '',
'Default' => '0',
'Extra' => '',
),
(object) array(
'Field' => 'created_at',
'Type' => 'timestamp',
'Null' => 'YES',
'Key' => '',
'Default' => null,
'Extra' => '',
),
),
$results
);

// now, no timestamp is expected
$this->assertQuery( 'INSERT INTO _tmp_table (id) VALUES (2)' );
$this->assertQuery( 'UPDATE _tmp_table SET id = 2 WHERE id = 2' );
$result = $this->assertQuery( 'SELECT * FROM _tmp_table WHERE id = 2' );
$this->assertNull( $result[0]->created_at );
}

public function testAlterTableWithColumnFirstAndAfter() {
$this->assertQuery(
"CREATE TABLE _tmp_table (
Expand Down
21 changes: 16 additions & 5 deletions wp-includes/sqlite/class-wp-sqlite-translator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,8 @@ private function parse_mysql_create_table_field() {
array( 'CURRENT_TIMESTAMP' )
)
) {
$this->rewriter->skip();
$this->rewriter->skip(); // ON UPDATE
$this->rewriter->skip(); // CURRENT_TIMESTAMP
$result->on_update = true;
continue;
}
Expand Down Expand Up @@ -3118,6 +3119,10 @@ private function execute_alter() {
$new_field->mysql_data_type
);

// Drop ON UPDATE trigger by the old column name.
$on_update_trigger_name = $this->get_column_on_update_current_timestamp_trigger_name( $this->table_name, $from_name );
$this->execute_sqlite_query( "DROP TRIGGER IF EXISTS \"$on_update_trigger_name\"" );

/*
* In SQLite, there is no direct equivalent to the CHANGE COLUMN
* statement from MySQL. We need to do a bit of work to emulate it.
Expand Down Expand Up @@ -3238,6 +3243,11 @@ private function execute_alter() {
);
}

// Add the ON UPDATE trigger if needed.
if ( $new_field->on_update ) {
$this->add_column_on_update_current_timestamp( $this->table_name, $new_field->name );
}

if ( ',' === $alter_terminator->token ) {
/*
* If the terminator was a comma,
Expand Down Expand Up @@ -3330,9 +3340,6 @@ private function execute_alter() {
);
$this->rewriter->drop_last();

$on_update_trigger_name = $this->get_column_on_update_current_timestamp_trigger_name( $this->table_name, $op_subject );
$this->execute_sqlite_query( "DROP TRIGGER IF EXISTS \"$on_update_trigger_name\"" );

$this->execute_sqlite_query(
$this->rewriter->get_updated_query()
);
Expand Down Expand Up @@ -4397,12 +4404,16 @@ private function generate_index_name( $table, $original_index_name ) {
*/
private function add_column_on_update_current_timestamp( $table, $column ) {
$trigger_name = $this->get_column_on_update_current_timestamp_trigger_name( $table, $column );

// The trigger wouldn't work for virtual and "WITHOUT ROWID" tables,
// but currently that can't happen as we're not creating such tables.
// See: https://www.sqlite.org/rowidtable.html
$this->execute_sqlite_query(
"CREATE TRIGGER \"$trigger_name\"
AFTER UPDATE ON \"$table\"
FOR EACH ROW
BEGIN
UPDATE \"$table\" SET \"$column\" = CURRENT_TIMESTAMP WHERE id = NEW.id;
UPDATE \"$table\" SET \"$column\" = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;
END"
);
}
Expand Down
Loading