Address PK issues for show create table queries (#127)

Fixes: #123 In this PR i propose a refactor of the handling of `show create table` queries. The main issue was that it is possible for a primary key to be composite (more than one column). The existing implementation only supports a single PK which causes the `CREATE TABLE` statement to be invalid. An example of such a table is the `wp_term_relationships` found in WordPress: ```mysql -- original MySQL statement CREATE TABLE `wp_term_relationships` ( `object_id` bigint(20) unsigned NOT NULL DEFAULT 0, `term_taxonomy_id` bigint(20) unsigned NOT NULL DEFAULT 0, `term_order` int(11) NOT NULL DEFAULT 0, PRIMARY KEY (`object_id`,`term_taxonomy_id`), KEY `term_taxonomy_id` (`term_taxonomy_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; -- statement produced by SQLite database integration CREATE TABLE wp_term_relationships ( `object_id` bigint(20) unsigned PRIMARY KEY AUTO_INCREMENT NOT NULL, `term_taxonomy_id` bigint(20) unsigned NOT NULL, `term_order` int(11) NOT NULL, KEY wp_term_relationships__term_taxonomy_id (term_taxonomy_id), UNIQUE KEY sqlite_autoindex_wp_term_relationships_1 (object_id, term_taxonomy_id) ); ``` SQLite also creates a unique key for any primary key (single or composite) which is something that is not necessary for MySQL and should be skipped from the final create statement. ## In this PR - Refactor the code to separate things into smaller methods. - Add the PK definition before the keys and remove it from the column definitions. - Use backticks around table names, column names and key identifiers. This is to prevent issues when names of identifiers are using reserved keywords, ensure case sensitivity and allow them to contain special chars or spaces. - Improve detection of AUTO_INCREMENT column. Now we falsely mark columns as AUTO_INCREMENT just because they happen to be the primary key and be of type integer. - Fix creating an invalid UNIQUE KEY for the PK. When we find that the unique key is identical to the primary key, we can skip the unique key as it is redundant. The final query output for `show create wp_term_relationships` looks as follows: ```mysql DROP TABLE IF EXISTS `wp_term_relationships`; CREATE TABLE `wp_term_relationships` ( `object_id` bigint(20) unsigned NOT NULL DEFAULT 0, `term_taxonomy_id` bigint(20) unsigned NOT NULL DEFAULT 0, `term_order` int(11) NOT NULL DEFAULT 0, PRIMARY KEY (`object_id`, `term_taxonomy_id`), KEY `wp_term_relationships__idx_term_taxonomy_id_x` (`term_taxonomy_id`) ); ``` ## Testing - Run the unit tests - Create a site with SQLite and then run `$wpdb->query('show create wp_term_relationships')` - The result should be a valid MySQL compatible query as shown above.
WordPress · Jul 19, 2024 · 9a80eb8 · 9a80eb8
1 parent d074dec
commit 9a80eb8
Show file tree

Hide file tree

Showing 2 changed files with 241 additions and 71 deletions.
diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php
@@ -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,
+			"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'}
 		);
@@ -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'}
 		);
@@ -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'}
 		);
@@ -358,16 +360,53 @@ 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'}
 		);
 	}
 
+	public function testShowCreateTableWithColumnKeys() {
+		$this->assertQuery(
+			"CREATE TABLE _tmp_table (
+	`ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL,
+	`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) );"
+		);
+	}
+
 	public function testSelectIndexHintForce() {
 		$this->assertQuery( "INSERT INTO _options (option_name) VALUES ('first');" );
 		$result = $this->assertQuery(

diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php
@@ -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;
@@ -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 ) . '")'
 			);
@@ -3345,56 +3347,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`.
@@ -3496,16 +3449,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 );
+
+		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 );
+	}
+
+	/**
+	 * 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.
+			$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',
@@ -3515,8 +3647,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 );
@@ -3535,9 +3667,8 @@ function ( $row ) use ( $name_map ) {
 				}
 				return $is_object ? (object) $new : $new;
 			},
-			$columns
+			$this->get_table_columns( $table_name )
 		);
-		return $columns;
 	}
 
 	/**