Skip to content
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.

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

Fix default null values and NULL value update edge case #94

Merged
merged 6 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 63 additions & 7 deletions tests/WP_SQLite_Translator_Tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ public function testCreateTable() {
'Type' => 'bigint(20) unsigned',
'Null' => 'NO',
'Key' => 'PRI',
'Default' => null,
'Default' => '0',
'Extra' => '',
),
(object) array(
Expand Down Expand Up @@ -1085,7 +1085,7 @@ public function testNestedTransactionWorkComplexModify() {
'Type' => 'integer',
'Null' => 'NO',
'Key' => 'PRI',
'Default' => null,
'Default' => '0',
'Extra' => '',
),
(object) array(
Expand Down Expand Up @@ -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;"
Expand All @@ -1995,5 +1996,60 @@ 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' => '2',
'name' => 'default-value',
'unique_name' => '2',
'inline_unique_name' => 'inline-unique-default-value',
'no_default' => '',
),
),
$result
);
}

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' => '',
'Default' => null,
'Extra' => '',
),
),
$result
);
}
}
20 changes: 17 additions & 3 deletions wp-includes/sqlite/class-wp-sqlite-translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1128,11 +1128,25 @@ 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 ) {
/**
* 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 if (in_array($field->sqlite_data_type, array('integer', 'real'), true)) {
$definition .= ' DEFAULT 0';
bgrgicak marked this conversation as resolved.
Show resolved Hide resolved
}
}

/*
Expand Down