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.

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

Address PK issues for show create table queries #127

Merged
merged 13 commits into from
Jul 19, 2024
Merged

Address PK issues for show create table queries #127

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6342f30
Address PK issues for show create table queries
jeroenpf Jul 17, 2024
a5e0f2c
Skip the unique key if it is equal to the PK
jeroenpf Jul 17, 2024
e3c0e60
Fix code style issues
jeroenpf Jul 17, 2024
2073f9d
Add additional check for autoindex
jeroenpf Jul 18, 2024
021438b
Merge branch 'main' into fix/create-table-pk-issues
jeroenpf Jul 18, 2024
a31fa98
Skip default value for auto increment columns
jeroenpf Jul 18, 2024
9c5d31b
Remove comment
jeroenpf Jul 18, 2024
b5bb9f5
Ensure PKs are ordered correctly in create statement
jeroenpf Jul 18, 2024
de96e2a
Expand the test case
jeroenpf Jul 18, 2024
80b4cfd
Fix linting issue
jeroenpf Jul 18, 2024
6d15515
Add comments to clarify methods
jeroenpf Jul 19, 2024
891cc63
Add a unit test to ensure queries with PKs at column level still work
jeroenpf Jul 19, 2024
1e4f8e3
Fix linting error
jeroenpf Jul 19, 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
54 changes: 41 additions & 13 deletions tests/WP_SQLite_Translator_Tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,13 @@ public function testShowCreateTable1() {
$results = $this->engine->get_query_results();
# TODO: Should we fix mismatch with original `option_value` text NOT NULL,` without default?
$this->assertEquals(
"CREATE TABLE _tmp_table (
`ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL,
bgrgicak marked this conversation as resolved.
Show resolved Hide resolved
"CREATE TABLE `_tmp_table` (
`ID` bigint NOT NULL AUTO_INCREMENT,
`option_name` varchar(255) DEFAULT '',
`option_value` text NOT NULL DEFAULT '',
KEY _tmp_table__composite (option_name, option_value),
UNIQUE KEY _tmp_table__option_name (option_name)
PRIMARY KEY (`ID`),
KEY `_tmp_table__composite` (`option_name`, `option_value`),
UNIQUE KEY `_tmp_table__option_name` (`option_name`)
);",
$results[0]->{'Create Table'}
);
Expand All @@ -306,12 +307,13 @@ public function testShowCreateTableQuoted() {
$results = $this->engine->get_query_results();
# TODO: Should we fix mismatch with original `option_value` text NOT NULL,` without default?
$this->assertEquals(
"CREATE TABLE _tmp_table (
`ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL,
"CREATE TABLE `_tmp_table` (
`ID` bigint NOT NULL AUTO_INCREMENT,
`option_name` varchar(255) DEFAULT '',
`option_value` text NOT NULL DEFAULT '',
KEY _tmp_table__composite (option_name, option_value),
UNIQUE KEY _tmp_table__option_name (option_name)
PRIMARY KEY (`ID`),
KEY `_tmp_table__composite` (`option_name`, `option_value`),
UNIQUE KEY `_tmp_table__option_name` (`option_name`)
);",
$results[0]->{'Create Table'}
);
Expand All @@ -329,8 +331,8 @@ public function testShowCreateTableSimpleTable() {
);
$results = $this->engine->get_query_results();
$this->assertEquals(
'CREATE TABLE _tmp_table (
`ID` bigint NOT NULL
'CREATE TABLE `_tmp_table` (
`ID` bigint NOT NULL DEFAULT 0
);',
$results[0]->{'Create Table'}
);
Expand Down Expand Up @@ -358,11 +360,37 @@ public function testShowCreateTableWithAlterAndCreateIndex() {
);
$results = $this->engine->get_query_results();
$this->assertEquals(
'CREATE TABLE _tmp_table (
`ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL,
'CREATE TABLE `_tmp_table` (
`ID` bigint NOT NULL AUTO_INCREMENT,
`option_name` smallint NOT NULL DEFAULT 14,
`option_value` text NOT NULL DEFAULT \'\',
KEY _tmp_table__option_name (option_name)
PRIMARY KEY (`ID`),
KEY `_tmp_table__option_name` (`option_name`)
);',
$results[0]->{'Create Table'}
);
}

public function testShowCreateTableWithPrimaryKeyColumnsReverseOrdered() {
$this->assertQuery(
'CREATE TABLE `_tmp_table` (
`ID_A` BIGINT NOT NULL,
`ID_B` BIGINT NOT NULL,
`ID_C` BIGINT NOT NULL,
PRIMARY KEY (`ID_B`, `ID_A`, `ID_C`)
);'
);

$this->assertQuery(
'SHOW CREATE TABLE _tmp_table;'
);
$results = $this->engine->get_query_results();
$this->assertEquals(
'CREATE TABLE `_tmp_table` (
`ID_A` bigint NOT NULL DEFAULT 0,
`ID_B` bigint NOT NULL DEFAULT 0,
`ID_C` bigint NOT NULL DEFAULT 0,
PRIMARY KEY (`ID_B`, `ID_A`, `ID_C`)
);',
$results[0]->{'Create Table'}
);
Expand Down
247 changes: 189 additions & 58 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 @@ -887,6 +887,7 @@ private function execute_create_table() {
implode( ",\n", $definitions ) .
')'
);

$this->execute_sqlite_query( $create_query );
$this->results = $this->last_exec_returned;
$this->return_value = $this->results;
Expand All @@ -898,6 +899,7 @@ private function execute_create_table() {
$unique = 'UNIQUE ';
}
$index_name = "{$table->name}__{$constraint->name}";

$this->execute_sqlite_query(
"CREATE $unique INDEX \"$index_name\" ON \"{$table->name}\" (\"" . implode( '", "', $constraint->columns ) . '")'
);
Expand Down Expand Up @@ -3339,56 +3341,7 @@ private function execute_show() {
return;

case 'CREATE TABLE':
// Value is unquoted table name
$table_name = $this->rewriter->consume()->value;
$columns = $this->get_columns_from( $table_name );
$keys = $this->get_keys( $table_name );

if ( empty( $columns ) ) {
$this->set_results_from_fetched_data(
array()
);
return;
}

foreach ( $columns as $column ) {
$column = (array) $column;
$definition = '';
$definition .= '`' . $column['Field'] . '` ';
$definition .= $this->get_cached_mysql_data_type(
$table_name,
$column['Field']
) ?? $column['Type'];
$definition .= 'PRI' === $column['Key'] ? ' PRIMARY KEY' : '';
$definition .= 'PRI' === $column['Key'] && 'INTEGER' === $column['Type'] ? ' AUTO_INCREMENT' : '';
$definition .= 'NO' === $column['Null'] ? ' NOT NULL' : '';
$definition .= $column['Default'] ? ' DEFAULT ' . $column['Default'] : '';
$entries[] = $definition;
}
foreach ( $keys as $key ) {
$key = (array) $key;
$definition = '';
$definition .= '1' === $key['index']['unique'] ? 'UNIQUE ' : '';
$definition .= 'KEY ';
$definition .= $key['index']['name'];
$definition .= ' (';
$definition .= implode(
', ',
array_column( $key['columns'], 'name' )
);
$definition .= ')';
$entries[] = $definition;
}
$create_table = "CREATE TABLE $table_name (\n\t";
$create_table .= implode( ",\n\t", $entries );
$create_table .= "\n);";
$this->set_results_from_fetched_data(
array(
(object) array(
'Create Table' => $create_table,
),
)
);
$this->generate_create_statement();
return;

case 'TABLE STATUS': // FROM `database`.
Expand Down Expand Up @@ -3490,16 +3443,195 @@ private function execute_show() {
}

/**
* Gets the columns from a table.
* Generates a MySQL compatible create statement for a SHOW CREATE TABLE query.
*
* @return void
*/
private function generate_create_statement() {
$table_name = $this->rewriter->consume()->value;
$columns = $this->get_table_columns( $table_name );
bgrgicak marked this conversation as resolved.
Show resolved Hide resolved

if ( empty( $columns ) ) {
$this->set_results_from_fetched_data( array() );
return;
}

$column_definitions = $this->get_column_definitions( $table_name, $columns );
$key_definitions = $this->get_key_definitions( $table_name, $columns );
$pk_definition = $this->get_primary_key_definition( $columns );

if ( $pk_definition ) {
array_unshift( $key_definitions, $pk_definition );
}

$sql_parts = array(
"CREATE TABLE `$table_name` (",
"\t" . implode( ",\n\t", array_merge( $column_definitions, $key_definitions ) ),
');',
);

$this->set_results_from_fetched_data(
array(
(object) array(
'Create Table' => implode( "\n", $sql_parts ),
),
)
);
}

/**
* Get raw columns details from pragma table info for the given table.
*
* @param string $table_name
*
* @return stdClass[]
*/
protected function get_table_columns( $table_name ) {
return $this->execute_sqlite_query( "PRAGMA table_info(\"$table_name\");" )
->fetchAll( $this->pdo_fetch_mode );
}
bgrgicak marked this conversation as resolved.
Show resolved Hide resolved

/**
* Get the column definitions for a create statement
*
* @param string $table_name
* @param array $columns
*
* @return array An array of column definitions
*/
protected function get_column_definitions( $table_name, $columns ) {
$auto_increment_column = $this->get_autoincrement_column( $table_name );
$column_definitions = array();
foreach ( $columns as $column ) {
$is_auto_incr = $auto_increment_column && strtolower( $auto_increment_column ) === strtolower( $column->name );
$definition = array();
$definition[] = '`' . $column->name . '`';
$definition[] = $this->get_cached_mysql_data_type( $table_name, $column->name ) ?? $column->name;

if ( '1' === $column->notnull ) {
$definition[] = 'NOT NULL';
}

if ( '' !== $column->dflt_value && ! $is_auto_incr ) {
$definition[] = 'DEFAULT ' . $column->dflt_value;
}

if ( $is_auto_incr ) {
$definition[] = 'AUTO_INCREMENT';
}
$column_definitions[] = implode( ' ', $definition );
}

return $column_definitions;
}

/**
* Get the key definitions for a create statement
*
* @param string $table_name
* @param array $columns
*
* @return array An array of key definitions
*/
private function get_key_definitions( $table_name, $columns ) {
$key_definitions = array();

$pks = array();
foreach ( $columns as $column ) {
if ( '0' !== $column->pk ) {
$pks[] = $column->name;
}
}

foreach ( $this->get_keys( $table_name ) as $key ) {
// If the PK columns are the same as the unique key columns, skip the key.
// This is because the PK is already unique in MySQL.
Comment on lines +3547 to +3548
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, these comments help a lot.

$key_equals_pk = ! array_diff( $pks, array_column( $key['columns'], 'name' ) );
$is_auto_index = strpos( $key['index']['name'], 'sqlite_autoindex_' ) === 0;
if ( $is_auto_index && $key['index']['unique'] && $key_equals_pk ) {
continue;
}

$key_definition = array();
if ( $key['index']['unique'] ) {
$key_definition[] = 'UNIQUE';
}

$key_definition[] = 'KEY';

$key_definition[] = sprintf( '`%s`', $key['index']['name'] );

$cols = array_map(
function ( $column ) {
return sprintf( '`%s`', $column['name'] );
},
$key['columns']
);

$key_definition[] = '(' . implode( ', ', $cols ) . ')';

$key_definitions[] = implode( ' ', $key_definition );
}

return $key_definitions;
}

/**
* Get the definition for the primary key(s) of a table.
*
* @param array $columns result from PRAGMA table_info() query
*
* @return string|null definition for the primary key(s)
*/
private function get_primary_key_definition( $columns ) {
$primary_keys = array();

// Sort the columns by primary key order.
usort(
$columns,
function ( $a, $b ) {
return $a->pk - $b->pk;
}
);

foreach ( $columns as $column ) {
if ( '0' !== $column->pk ) {
$primary_keys[] = sprintf( '`%s`', $column->name );
}
}

return ! empty( $primary_keys )
? sprintf( 'PRIMARY KEY (%s)', implode( ', ', $primary_keys ) )
: null;
}

/**
* Get the auto-increment column from a table.
*
* @param $table_name
*
* @return string|null
*/
private function get_autoincrement_column( $table_name ) {
preg_match(
'/"([^"]+)"\s+integer\s+primary\s+key\s+autoincrement/i',
$this->get_sqlite_create_table( $table_name ),
$matches
);

return $matches[1] ?? null;
}

/**
* Gets the columns from a table for the SHOW COLUMNS query.
*
* The output is identical to the output of the MySQL `SHOW COLUMNS` query.
*
* @param string $table_name The table name.
*
* @return array The columns.
*/
private function get_columns_from( $table_name ) {
$stmt = $this->execute_sqlite_query(
"PRAGMA table_info(\"$table_name\");"
);
/* @todo we may need to add the Extra column if anybdy needs it. 'auto_increment' is the value */
$name_map = array(
'name' => 'Field',
Expand All @@ -3509,8 +3641,8 @@ private function get_columns_from( $table_name ) {
'notnull' => null,
'pk' => null,
);
$columns = $stmt->fetchAll( $this->pdo_fetch_mode );
$columns = array_map(

return array_map(
function ( $row ) use ( $name_map ) {
$new = array();
$is_object = is_object( $row );
Expand All @@ -3529,9 +3661,8 @@ function ( $row ) use ( $name_map ) {
}
return $is_object ? (object) $new : $new;
},
$columns
$this->get_table_columns( $table_name )
);
return $columns;
}

/**
Expand Down
Loading