diff --git a/README.md b/README.md index 3d762fb..de29bc7 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,13 @@ class MyMigration extends Migration use \OrisIntel\OnlineMigrator\InnodbOnlineDdl ``` +Do not combine queries for a migration when using PTOSC: +``` php +class MyMigration extends Migration +{ + use \OrisIntel\OnlineMigrator\CombineIncompatible +``` + Adding foreign key with index to existing table: ``` php class MyColumnWithFkMigration extends Migration diff --git a/src/CombineIncompatible.php b/src/CombineIncompatible.php new file mode 100644 index 0000000..826513d --- /dev/null +++ b/src/CombineIncompatible.php @@ -0,0 +1,9 @@ +combineIncompatible ?? false); } /** diff --git a/src/Strategy/InnodbOnlineDdl.php b/src/Strategy/InnodbOnlineDdl.php index 635e0d8..7442df5 100644 --- a/src/Strategy/InnodbOnlineDdl.php +++ b/src/Strategy/InnodbOnlineDdl.php @@ -4,7 +4,7 @@ use Illuminate\Database\Connection; -class InnodbOnlineDdl implements StrategyInterface +final class InnodbOnlineDdl implements StrategyInterface { private const INPLACE_INCOMPATIBLE = [ 'ALTER\s+TABLE\s+`?[^`\s]+`?\s+(CHANGE|MODIFY)', // CONSIDER: Only when type changes. @@ -13,15 +13,33 @@ class InnodbOnlineDdl implements StrategyInterface // Foreign keys depend upon state of foreign_key_checks. ]; + /** + * Get queries and commands, converting "ALTER TABLE " statements to on-line commands/queries. + * + * @param array $queries + * @param array $connection + * @param bool $combineIncompatible + * + * @return array of queries and--where supported--commands + */ + public static function getQueriesAndCommands(array &$queries, Connection $connection, bool $combineIncompatible = false) : array + { + foreach ($queries as &$query) { + $query['query'] = self::getQueryOrCommand($query, $connection); + } + + return $queries; + } + /** * Get query or command, converting "ALTER TABLE " statements to on-line commands/queries. * * @param array $query - * @param array $db_config + * @param array $connection * * @return string */ - public static function getQueryOrCommand(array &$query, Connection $connection) + public static function getQueryOrCommand(array &$query, Connection $connection) : string { $query_or_command_str = rtrim($query['query'], '; '); @@ -106,7 +124,7 @@ private static function isInplaceCompatible(string $query_str, Connection $conne * * @return void */ - public static function runQueryOrCommand(array &$query, Connection $connection) + public static function runQueryOrCommand(array &$query, Connection $connection) : void { // Always run unchanged query since this strategy does not need to // execute commands of other tools. diff --git a/src/Strategy/PtOnlineSchemaChange.php b/src/Strategy/PtOnlineSchemaChange.php index 0701024..1b9e8a6 100644 --- a/src/Strategy/PtOnlineSchemaChange.php +++ b/src/Strategy/PtOnlineSchemaChange.php @@ -10,90 +10,219 @@ use Illuminate\Database\Connection; -class PtOnlineSchemaChange implements StrategyInterface +final class PtOnlineSchemaChange implements StrategyInterface { /** - * Get query or command, converting "ALTER TABLE " statements to on-line commands/queries. + * Get queries and commands, converting "ALTER TABLE " statements to on-line commands/queries. * - * @param array $query - * @param array $db_config + * @param array $queries + * @param array $connection + * @param bool $combineIncompatible * - * @return string + * @return array of queries and--where supported--commands */ - public static function getQueryOrCommand(array &$query, Connection $connection) + public static function getQueriesAndCommands(array &$queries, Connection $connection, bool $combineIncompatible = false) : array { - $query_or_command_str = $query['query']; - // CONSIDER: Executing --dry-run (only during pretend?) first to validate all will work. + /*** @var array like ['table_name' => string, 'changes' => array]. */ + $combining = []; + + $queries_commands = []; + foreach ($queries as &$query) { + if ( + ! $combineIncompatible + && $combinable = self::getCombinableParts($query) + ) { + // First adjacent combinable. + if (empty($combining)) { + $combining = $combinable; + continue; + } + + // Same table, so combine changes into comma-separated string. + if ($combining['table_name'] === $combinable['table_name']) { + $combining['changes'] = + (!empty($combining['changes']) ? $combining['changes'] . ', ' : '') + . $combinable['changes']; + continue; + } + + // Different table, so store previous combinables and reset. + $queries_commands[] = self::getCombinedWithBindings($combining, $connection); + $combining = $combinable; + continue; + } + + // Not combinable, so store any previous combinables. + if (! empty($combining)) { + $queries_commands[] = self::getCombinedWithBindings($combining, $connection); + $combining = []; + } + + $query['query'] = self::getQueryOrCommand($query, $connection); + $queries_commands[] = $query; + } + + // Store residual combinables so they aren't lost. + if (! empty($combining)) { + $queries_commands[] = self::getCombinedWithBindings($combining, $connection); + } + return $queries_commands; + } + + /** + * @param array $combining like ['table_name' => string, 'changes' => string] + * @param Connection $connection + * + * @return array like ['query' => string, 'binding' => array, 'time' => float]. + */ + private static function getCombinedWithBindings(array $combining, Connection $connection) : array + { + $query_bindings_time = [ + 'query' => "ALTER TABLE $combining[escape]$combining[table_name]$combining[escape] $combining[changes]", + 'bindings' => [], + 'time' => 0.0, + ]; + $query_bindings_time['query'] = self::getQueryOrCommand($query_bindings_time, $connection); + + return $query_bindings_time; + } + + /** + * @return array like 'table_name' => string, 'changes' => string]. + */ + public static function getCombinableParts(array $query) : array + { + // CONSIDER: Combining if all named or all unnamed. + if (! empty($query['bindings'])) { + return []; + } + + $parts = self::getOnlineableParts($query['query']); + + // CONSIDER: Supporting combinable partition options. + if (preg_match('/\A\s*(ADD|ALTER|DROP|CHANGE)\b/imu', $parts['changes'] ?? '')) { + return $parts; + } + + return []; + } + + /** + * @param string $query_string + * + * @return array like ['table_name' => ?string, 'changes' => ?string]. + */ + private static function getOnlineableParts(string $query_string) : array + { $table_name = null; $changes = null; + $escape = null; - $alter_re = '/\A\s*ALTER\s+TABLE\s+[`"]?([^\s`"]+)[`"]?\s*/imu'; + $alter_re = '/\A\s*ALTER\s+TABLE\s+([`"]?[^\s`"]+[`"]?)\s*/imu'; $create_re = '/\A\s*CREATE\s+' . '((UNIQUE|FULLTEXT|SPATIAL)\s+)?' - . 'INDEX\s+[`"]?([^`"\s]+)[`"]?\s+ON\s+[`"]?([^`"\s]+)[`"]?\s+?/imu'; + . 'INDEX\s+[`"]?([^`"\s]+)[`"]?\s+ON\s+([`"]?[^`"\s]+[`"]?)\s+?/imu'; $drop_re = '/\A\s*DROP\s+' - . 'INDEX\s+[`"]?([^`"\s]+)[`"]?\s+ON\s+[`"]?([^`"\s]+)[`"]?\s*?/imu'; - if (preg_match($alter_re, $query_or_command_str, $alter_parts)) { + . 'INDEX\s+[`"]?([^`"\s]+)[`"]?\s+ON\s+([`"]?[^`"\s]+[`"]?)\s*?/imu'; + if (preg_match($alter_re, $query_string, $alter_parts)) { $table_name = $alter_parts[1]; // Changing query so pretendToRun output will match command. // CONSIDER: Separate index and overriding pretendToRun instead. - $changes = preg_replace($alter_re, '', $query_or_command_str); - } elseif (preg_match($create_re, $query_or_command_str, $create_parts)) { + $changes = preg_replace($alter_re, '', $query_string); + + // Alter-table-rename-to-as is not supported by PTOSC. + if (preg_match('/\A\s*RENAME(\s+(TO|AS))?\s+[^\s]+\s*(;|\z)/imu', $changes)) { + return []; + } + } elseif (preg_match($create_re, $query_string, $create_parts)) { $index_name = $create_parts[3]; $table_name = $create_parts[4]; $changes = "ADD $create_parts[2] INDEX $index_name " - . preg_replace($create_re, '', $query_or_command_str); - } elseif (preg_match($drop_re, $query_or_command_str, $drop_parts)) { + . preg_replace($create_re, '', $query_string); + } elseif (preg_match($drop_re, $query_string, $drop_parts)) { $index_name = $drop_parts[1]; $table_name = $drop_parts[2]; $changes = "DROP INDEX $index_name " - . preg_replace($drop_re, '', $query_or_command_str); + . preg_replace($drop_re, '', $query_string); + } else { + // Query not supported by PTOSC. + return []; } - if ($table_name && $changes) { - // HACK: Workaround PTOSC quirk with escaping and defaults. - $changes = str_replace( - ["default '0'", "default '1'"], - ['default 0', 'default 1'], $changes); - - // Dropping FKs with PTOSC requires prefixing constraint name with - // '_'; adding another if it already starts with '_'. - $changes = preg_replace('/(\bDROP\s+FOREIGN\s+KEY\s+[`"]?)([^`"\s]+)/imu', '\01_\02', $changes); - - // Keeping defaults here so overriding one does not discard all, as - // would happen if left to `config/online-migrator.php`. - $ptosc_defaults = [ - '--alter-foreign-keys-method=auto', - '--no-check-alter', // ASSUMES: Users accept risks w/RENAME. - // ASSUMES: All are known to be unique. - // CONSIDER: Extracting/re-creating automatic uniqueness checks - // and running them here in PHP beforehand. - '--no-check-unique-key-change', - ]; - $ptosc_options_str = self::getOptionsForShell( - config('online-migrator.ptosc-options'), $ptosc_defaults); - - if (false !== strpos($ptosc_options_str, '--dry-run')) { - throw new \InvalidArgumentException( - 'Cannot run PTOSC with --dry-run because it would incompletely change the database. Remove from PTOSC_OPTIONS.'); - } + $escape = preg_match('/^([`"])/u', $table_name, $m) ? $m[1] : null; + $table_name = trim($table_name, '`"'); + + // HACK: Workaround PTOSC quirk with escaping and defaults. + $changes = str_replace( + ["default '0'", "default '1'"], + ['default 0', 'default 1'], $changes); + + // Dropping FKs with PTOSC requires prefixing constraint name with + // '_'; adding another if it already starts with '_'. + $changes = preg_replace('/(\bDROP\s+FOREIGN\s+KEY\s+[`"]?)([^`"\s]+)/imu', '\01_\02', $changes); + + return [ + 'table_name' => $table_name, + 'changes' => $changes, + 'escape' => $escape, + ]; + } - $db_config = $connection->getConfig(); - $query_or_command_str = 'pt-online-schema-change --alter ' - . escapeshellarg($changes) - . ' D=' . escapeshellarg($db_config['database'] . ',t=' . $table_name) - . ' --host ' . escapeshellarg($db_config['host']) - . ' --port ' . escapeshellarg($db_config['port']) - . ' --user ' . escapeshellarg($db_config['username']) - // CONSIDER: Redacting password during pretend - . ' --password ' . escapeshellarg($db_config['password']) - . $ptosc_options_str - . ' --execute' - . ' 2>&1'; + /** + * Get query or command, converting "ALTER TABLE " statements to on-line commands/queries. + * + * @param array $query + * @param array $connection + * + * @return string + */ + public static function getQueryOrCommand(array &$query, Connection $connection) : string + { + // CONSIDER: Executing --dry-run (only during pretend?) first to validate all will work. + + $onlineable = self::getOnlineableParts($query['query']); + + // Leave unchanged when not supported by PTOSC. + if ( + empty($onlineable['table_name']) + || empty($onlineable['changes']) + ) { + return $query['query']; } - return $query_or_command_str; + // Keeping defaults here so overriding one does not discard all, as + // would happen if left to `config/online-migrator.php`. + $ptosc_defaults = [ + '--alter-foreign-keys-method=auto', + '--no-check-alter', // ASSUMES: Users accept risks w/RENAME. + // ASSUMES: All are known to be unique. + // CONSIDER: Extracting/re-creating automatic uniqueness checks + // and running them here in PHP beforehand. + '--no-check-unique-key-change', + ]; + $ptosc_options_str = self::getOptionsForShell( + config('online-migrator.ptosc-options'), $ptosc_defaults); + + if (false !== strpos($ptosc_options_str, '--dry-run')) { + throw new \InvalidArgumentException( + 'Cannot run PTOSC with --dry-run because it would incompletely change the database. Remove from PTOSC_OPTIONS.'); + } + + $db_config = $connection->getConfig(); + $command = 'pt-online-schema-change --alter ' + . escapeshellarg($onlineable['changes']) + . ' D=' . escapeshellarg($db_config['database'] . ',t=' . $onlineable['table_name']) + . ' --host ' . escapeshellarg($db_config['host']) + . ' --port ' . escapeshellarg($db_config['port']) + . ' --user ' . escapeshellarg($db_config['username']) + // CONSIDER: Redacting password during pretend + . ' --password ' . escapeshellarg($db_config['password']) + . $ptosc_options_str + . ' --execute' + . ' 2>&1'; + + return $command; } /** @@ -153,7 +282,7 @@ public static function getOptionsForShell(?string $option_csv, array $defaults = * * @return void */ - public static function runQueryOrCommand(array &$query, Connection $connection) + public static function runQueryOrCommand(array &$query, Connection $connection) : void { // CONSIDER: Using unmodified migration code when small and not // currently locked table. diff --git a/src/Strategy/StrategyInterface.php b/src/Strategy/StrategyInterface.php index 9eaac16..55a0750 100644 --- a/src/Strategy/StrategyInterface.php +++ b/src/Strategy/StrategyInterface.php @@ -7,14 +7,15 @@ interface StrategyInterface { /** - * Get query or command, converting "ALTER TABLE " statements to on-line commands/queries. + * Get queries and commands, converting "ALTER TABLE " statements to on-line commands/queries. * - * @param array $query - * @param array $db_config + * @param array $queries + * @param array $connection + * @param bool $combineIncompatible * - * @return string + * @return array of queries and--where supported--commands */ - public static function getQueryOrCommand(array &$query, Connection $connection); + public static function getQueriesAndCommands(array &$queries, Connection $connection, bool $combineIncompatible = false) : array; /** * Execute query or on-line command. @@ -26,5 +27,5 @@ public static function getQueryOrCommand(array &$query, Connection $connection); * * @return void */ - public static function runQueryOrCommand(array &$query, Connection $connection); + public static function runQueryOrCommand(array &$query, Connection $connection) : void; } diff --git a/tests/PtOnlineSchemaChangeTest.php b/tests/PtOnlineSchemaChangeTest.php index 0864912..1da696b 100644 --- a/tests/PtOnlineSchemaChangeTest.php +++ b/tests/PtOnlineSchemaChangeTest.php @@ -18,6 +18,14 @@ public function test_getOptionsForShell_overridesDefault() PtOnlineSchemaChange::getOptionsForShell('--alter-foreign-keys-method=none', ['--alter-foreign-keys-method=auto'])); } + public function test_getQueryOrCommand_doesntRewriteTableRename() + { + $query = ['query' => 'ALTER TABLE `t` RENAME `t2`']; + + $query_or_command = PtOnlineSchemaChange::getQueryOrCommand($query, \DB::connection()); + $this->assertEquals($query['query'], $query_or_command); + } + public function test_getQueryOrCommand_rewritesDropForeignKey() { $query = ['query' => 'ALTER TABLE t DROP FOREIGN KEY fk, DROP FOREIGN KEY fk2']; @@ -125,6 +133,36 @@ public function test_migrate_createsTableWithPrimary() \DB::table('test_om_with_primary')->insert(['name' => 'alice']); } + public function test_migrate_combinesAdjacentDdl() + { + $queries = [ + ['query' => 'ALTER TABLE t ADD c INT'], + ['query' => 'ALTER TABLE t ALTER c2 SET DEFAULT 0'], + ['query' => 'ALTER TABLE t DROP c3'], + ['query' => 'ALTER TABLE t CHANGE c4 c4 TEXT'], + ['query' => 'ALTER TABLE t2 ADD c INT'], + ['query' => 'ALTER TABLE t2 ADD c2 INT'], + ]; + + $converted = PtOnlineSchemaChange::getQueriesAndCommands($queries, \DB::connection()); + $this->assertCount(2, $converted); + $this->assertStringStartsWith('pt-online-schema-change', $converted[0]['query']); + $this->assertContains('ADD c INT, ALTER c2 SET DEFAULT 0, DROP c3, CHANGE c4 c4 TEXT', $converted[0]['query']); + $this->assertContains('ADD c INT, ADD c2 INT', $converted[1]['query']); + } + + public function test_migrate_doesNotCombineUnsupportedSql() + { + $queries = [ + ['query' => 'ALTER TABLE t ADD c INT'], + ['query' => 'ALTER TABLE t EXCHANGE PARTITION p WITH TABLE t2'], + ]; + + $converted = PtOnlineSchemaChange::getQueriesAndCommands($queries, \DB::connection()); + $this->assertCount(2, $converted); + $this->assertNotContains(", EXCHANGE PARTITION", $converted[0]['query']); + } + public function test_migrate_dropsIndexWithSql() { $this->loadMigrationsFrom(__DIR__ . '/migrations/creates-index-with-raw-sql'); diff --git a/tests/migrations/creates-fk-with-index/0000_00_00_000001_create_fk_with_index.php b/tests/migrations/creates-fk-with-index/0000_00_00_000001_create_fk_with_index.php index beff334..c3be7f0 100644 --- a/tests/migrations/creates-fk-with-index/0000_00_00_000001_create_fk_with_index.php +++ b/tests/migrations/creates-fk-with-index/0000_00_00_000001_create_fk_with_index.php @@ -2,6 +2,8 @@ class CreateFkWithIndex extends \Illuminate\Database\Migrations\Migration { + use \OrisIntel\OnlineMigrator\CombineIncompatible; + public function up() { Schema::create('test_om_fk_with_index', function (\Illuminate\Database\Schema\Blueprint $table) {