From c2238e3ffbcfff5a0a21673b2090a5369ece61ef Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Thu, 11 Apr 2024 13:45:55 +0200 Subject: [PATCH 1/6] Add DEFAULT NULL support --- wp-includes/sqlite/class-wp-sqlite-translator.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 34ad86b7..285f8c5e 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1128,10 +1128,12 @@ private function make_sqlite_field_definition( $field ) { * This mode allows the use of `NULL` when NOT NULL is set on a column that falls back to DEFAULT. * SQLite does not support this behavior, so we need to add the `ON CONFLICT REPLACE` clause to the column definition. */ - if (null !== $field->default && $field->not_null) { + if ($field->not_null) { $definition .= ' ON CONFLICT REPLACE'; } - if ( null !== $field->default ) { + if (null === $field->default) { + $definition .= ' DEFAULT NULL'; + } else if (false !== $field->default) { $definition .= ' DEFAULT ' . $field->default; } From 7808ceab9c59c0c82dff6de3648507aae0ea0893 Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Thu, 11 Apr 2024 13:46:17 +0200 Subject: [PATCH 2/6] Add broken tests --- tests/WP_SQLite_Translator_Tests.php | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index cfebc685..4886aafa 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -1996,4 +1996,40 @@ public function testOnConflictReplace() '' ); } + + public function testDefaultNullValue() + { + $this->assertQuery( + "CREATE TABLE _tmp_table ( + name varchar(20) NOT NULL default NULL, + no_default varchar(20) NOT NULL + );" + ); + + $result = $this->assertQuery( + "DESCRIBE _tmp_table;" + ); + $this->assertEquals( + array( + (object) array( + 'Field' => 'name', + 'Type' => 'varchar(20)', + 'Null' => 'NO', + 'Key' => '', + 'Default' => 'NULL', + 'Extra' => '', + ), + (object) array( + 'Field' => 'no_default', + 'Type' => 'varchar(20)', + 'Null' => 'NO', + 'Key' => '', + // There is still a bug here if there is no default value Default should be empty of false + 'Default' => 'NULL', + 'Extra' => '', + ), + ), + $result + ); + } } From 99b0c6c7b28151c88c05355273df5ef8f2c5fb97 Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Mon, 15 Apr 2024 10:01:14 +0200 Subject: [PATCH 3/6] Update default to be false if it doesn't exist --- tests/WP_SQLite_Translator_Tests.php | 3 +-- wp-includes/sqlite/class-wp-sqlite-translator.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index 4886aafa..a23277bb 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -2024,8 +2024,7 @@ public function testDefaultNullValue() 'Type' => 'varchar(20)', 'Null' => 'NO', 'Key' => '', - // There is still a bug here if there is no default value Default should be empty of false - 'Default' => 'NULL', + 'Default' => null, 'Extra' => '', ), ), diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 285f8c5e..f5c4437a 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1040,7 +1040,7 @@ private function parse_mysql_create_table_field() { $result->name = ''; $result->sqlite_data_type = ''; $result->not_null = false; - $result->default = null; + $result->default = false; $result->auto_increment = false; $result->primary_key = false; From c5606dd55368b4e610705ebd11aa71dbf41acdc8 Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Mon, 15 Apr 2024 10:17:50 +0200 Subject: [PATCH 4/6] Update tests to include all cases --- tests/WP_SQLite_Translator_Tests.php | 31 +++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index a23277bb..e674a119 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -1946,29 +1946,30 @@ public function testOnConflictReplace() name varchar(20) NOT NULL default 'default-value', unique_name varchar(20) NOT NULL default 'unique-default-value', inline_unique_name varchar(20) NOT NULL default 'inline-unique-default-value', + no_default varchar(20) NOT NULL, UNIQUE KEY unique_name (unique_name) );" ); $this->assertQuery( - "INSERT INTO _tmp_table (ID, name, unique_name, inline_unique_name) VALUES (1, null, null, null);" + "INSERT INTO _tmp_table VALUES (1, null, null, null, '');" ); - $result = $this->assertQuery("SELECT name, unique_name, inline_unique_name FROM _tmp_table"); - - $result = $this->assertQuery("SELECT name, unique_name, inline_unique_name FROM _tmp_table"); + $result = $this->assertQuery("SELECT * FROM _tmp_table WHERE ID = 1"); $this->assertEquals( array( (object) array( + 'ID' => '1', 'name' => 'default-value', 'unique_name' => 'unique-default-value', 'inline_unique_name' => 'inline-unique-default-value', + 'no_default' => '', ), ), $result ); $this->assertQuery( - "INSERT INTO _tmp_table (ID, name, unique_name, inline_unique_name) VALUES (2, '1', '2', '3');" + "INSERT INTO _tmp_table VALUES (2, '1', '2', '3', '4');" ); $this->assertQuery( "UPDATE _tmp_table SET name = null WHERE ID = 2;" @@ -1995,6 +1996,26 @@ public function testOnConflictReplace() "UPDATE _tmp_table SET inline_unique_name = NULL WHERE ID = 2;", '' ); + + // WPDB allows for NULL values in columns that don't have a default value and a NOT NULL constraint + $this->assertQuery( + "UPDATE _tmp_table SET no_default = NULL WHERE ID = 2;", + '' + ); + + $result = $this->assertQuery("SELECT * FROM _tmp_table WHERE ID = 2"); + $this->assertEquals( + array( + (object) array( + 'ID' => '1', + 'name' => 'default-value', + 'unique_name' => 'unique-default-value', + 'inline_unique_name' => 'inline-unique-default-value', + 'no_default' => '', + ), + ), + $result + ); } public function testDefaultNullValue() From 70d6b8c3e667fe8590a8e639865045c64757a29b Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Mon, 15 Apr 2024 11:30:47 +0200 Subject: [PATCH 5/6] Ensure default always exists --- tests/WP_SQLite_Translator_Tests.php | 8 ++++---- wp-includes/sqlite/class-wp-sqlite-translator.php | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index e674a119..faa78669 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -368,7 +368,7 @@ public function testCreateTable() { 'Type' => 'bigint(20) unsigned', 'Null' => 'NO', 'Key' => 'PRI', - 'Default' => null, + 'Default' => '0', 'Extra' => '', ), (object) array( @@ -1085,7 +1085,7 @@ public function testNestedTransactionWorkComplexModify() { 'Type' => 'integer', 'Null' => 'NO', 'Key' => 'PRI', - 'Default' => null, + 'Default' => '0', 'Extra' => '', ), (object) array( @@ -2007,9 +2007,9 @@ public function testOnConflictReplace() $this->assertEquals( array( (object) array( - 'ID' => '1', + 'ID' => '2', 'name' => 'default-value', - 'unique_name' => 'unique-default-value', + 'unique_name' => '2', 'inline_unique_name' => 'inline-unique-default-value', 'no_default' => '', ), diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index f5c4437a..5b918678 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1135,6 +1135,12 @@ private function make_sqlite_field_definition( $field ) { $definition .= ' DEFAULT NULL'; } else if (false !== $field->default) { $definition .= ' DEFAULT ' . $field->default; + } else if ($field->not_null) { + if ('text' === $field->sqlite_data_type) { + $definition .= ' DEFAULT \'\''; + } else { + $definition .= ' DEFAULT 0'; + } } /* From 994609207e6c185fb4fd2c272d9b4ab5ba841351 Mon Sep 17 00:00:00 2001 From: bgrgicak Date: Mon, 15 Apr 2024 13:34:04 +0200 Subject: [PATCH 6/6] Specify types and add comments --- wp-includes/sqlite/class-wp-sqlite-translator.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 5b918678..9a1fabc6 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1131,14 +1131,20 @@ private function make_sqlite_field_definition( $field ) { if ($field->not_null) { $definition .= ' ON CONFLICT REPLACE'; } + /** + * The value of DEFAULT can be NULL. PHP would print this as an empty string, so we need a special case for it. + */ if (null === $field->default) { $definition .= ' DEFAULT NULL'; } else if (false !== $field->default) { $definition .= ' DEFAULT ' . $field->default; } else if ($field->not_null) { + /** + * If the column is NOT NULL, we need to provide a default value to match WPDB behavior caused by removing the STRICT_TRANS_TABLES mode. + */ if ('text' === $field->sqlite_data_type) { $definition .= ' DEFAULT \'\''; - } else { + } else if (in_array($field->sqlite_data_type, array('integer', 'real'), true)) { $definition .= ' DEFAULT 0'; } }