diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php --- a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php @@ -32,7 +32,7 @@ self::CONFIG_COLUMN_SCHEMA => array( 'isEnabled' => 'bool', 'providerClass' => 'text128', - 'providerType' => 'text64', + 'providerType' => 'text32', 'providerDomain' => 'text128', 'shouldAllowLogin' => 'bool', 'shouldAllowRegistration' => 'bool', diff --git a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php --- a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php +++ b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php @@ -15,7 +15,7 @@ self::CONFIG_COLUMN_SCHEMA => array( 'id' => 'id64', 'connectionID' => 'id64?', - 'method' => 'text255', + 'method' => 'text64', 'error' => 'text255', 'duration' => 'uint64', 'callerPHID' => 'phid?', diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseController.php b/src/applications/config/controller/PhabricatorConfigDatabaseController.php --- a/src/applications/config/controller/PhabricatorConfigDatabaseController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseController.php @@ -3,8 +3,6 @@ abstract class PhabricatorConfigDatabaseController extends PhabricatorConfigController { - const MAX_INNODB_KEY_LENGTH = 767; - protected function buildSchemaQuery() { $conf = PhabricatorEnv::newObjectFromConfig( 'mysql.configuration-provider', diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php --- a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php @@ -281,6 +281,7 @@ $nullable_issue = PhabricatorConfigStorageSchema::ISSUE_NULLABLE; $unique_issue = PhabricatorConfigStorageSchema::ISSUE_UNIQUE; $columns_issue = PhabricatorConfigStorageSchema::ISSUE_KEYCOLUMNS; + $longkey_issue = PhabricatorConfigStorageSchema::ISSUE_LONGKEY; $database = $comp->getDatabase($database_name); if (!$database) { @@ -392,7 +393,7 @@ if ($size) { $size_formatted = $this->renderAttr( $size, - ($size > self::MAX_INNODB_KEY_LENGTH)); + $key->hasIssue($longkey_issue)); } $key_rows[] = array( diff --git a/src/applications/config/schema/PhabricatorConfigColumnSchema.php b/src/applications/config/schema/PhabricatorConfigColumnSchema.php --- a/src/applications/config/schema/PhabricatorConfigColumnSchema.php +++ b/src/applications/config/schema/PhabricatorConfigColumnSchema.php @@ -62,7 +62,7 @@ $type = $this->getColumnType(); $matches = null; - if (preg_match('/^varchar\((\d+)\)$/', $type, $matches)) { + if (preg_match('/^(?:var)?char\((\d+)\)$/', $type, $matches)) { // For utf8mb4, each character requires 4 bytes. $size = (int)$matches[1]; if ($prefix && $prefix < $size) { @@ -72,8 +72,8 @@ } $matches = null; - if (preg_match('/^char\((\d+)\)$/', $type, $matches)) { - // We use char() only for fixed-length binary data, so its size + if (preg_match('/^(?:var)?binary\((\d+)\)$/', $type, $matches)) { + // binary()/varbinary() store fixed-length binary data, so their size // is always the column size. $size = (int)$matches[1]; if ($prefix && $prefix < $size) { diff --git a/src/applications/config/schema/PhabricatorConfigKeySchema.php b/src/applications/config/schema/PhabricatorConfigKeySchema.php --- a/src/applications/config/schema/PhabricatorConfigKeySchema.php +++ b/src/applications/config/schema/PhabricatorConfigKeySchema.php @@ -3,8 +3,30 @@ final class PhabricatorConfigKeySchema extends PhabricatorConfigStorageSchema { + const MAX_INNODB_KEY_LENGTH = 767; + private $columnNames; private $unique; + private $table; + private $indexType; + + public function setIndexType($index_type) { + $this->indexType = $index_type; + return $this; + } + + public function getIndexType() { + return $this->indexType; + } + + public function setProperty($property) { + $this->property = $property; + return $this; + } + + public function getProperty() { + return $this->property; + } public function setUnique($unique) { $this->unique = $unique; @@ -15,6 +37,15 @@ return $this->unique; } + public function setTable(PhabricatorConfigTableSchema $table) { + $this->table = $table; + return $this; + } + + public function getTable() { + return $this->table; + } + public function setColumnNames(array $column_names) { $this->columnNames = array_values($column_names); return $this; @@ -37,6 +68,21 @@ } } + public function getKeyByteLength() { + $size = 0; + foreach ($this->getColumnNames() as $column_spec) { + list($column_name, $prefix) = $this->getKeyColumnAndPrefix($column_spec); + $column = $this->getTable()->getColumn($column_name); + if (!$column) { + $size = 0; + break; + } + $size += $column->getKeyByteLength($prefix); + } + + return $size; + } + public function compareToSimilarSchema( PhabricatorConfigStorageSchema $expect) { @@ -49,11 +95,19 @@ $issues[] = self::ISSUE_UNIQUE; } + // A fulltext index can be of any length. + if ($this->getIndexType() != 'FULLTEXT') { + if ($this->getKeyByteLength() > self::MAX_INNODB_KEY_LENGTH) { + $issues[] = self::ISSUE_LONGKEY; + } + } + return $issues; } public function newEmptyClone() { $clone = clone $this; + $this->table = null; return $clone; } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php --- a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php @@ -129,7 +129,8 @@ $key_schema = id(new PhabricatorConfigKeySchema()) ->setName($key_name) ->setColumnNames($column_names) - ->setUnique(!$head['Non_unique']); + ->setUnique(!$head['Non_unique']) + ->setIndexType($head['Index_type']); $table_schema->addKey($key_schema); } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php --- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php @@ -114,6 +114,7 @@ ->setColumnNames(idx($key_spec, 'columns', array())); $key->setUnique((bool)idx($key_spec, 'unique')); + $key->setIndexType(idx($key_spec, 'type', 'BTREE')); $table->addKey($key); } diff --git a/src/applications/config/schema/PhabricatorConfigStorageSchema.php b/src/applications/config/schema/PhabricatorConfigStorageSchema.php --- a/src/applications/config/schema/PhabricatorConfigStorageSchema.php +++ b/src/applications/config/schema/PhabricatorConfigStorageSchema.php @@ -12,6 +12,7 @@ const ISSUE_NULLABLE = 'nullable'; const ISSUE_KEYCOLUMNS = 'keycolumns'; const ISSUE_UNIQUE = 'unique'; + const ISSUE_LONGKEY = 'longkey'; const ISSUE_SUBWARN = 'subwarn'; const ISSUE_SUBFAIL = 'subfail'; @@ -117,6 +118,8 @@ return pht('Key on Wrong Columns'); case self::ISSUE_UNIQUE: return pht('Key has Wrong Uniqueness'); + case self::ISSUE_LONGKEY: + return pht('Key is Too Long'); case self::ISSUE_SUBWARN: return pht('Subschemata Have Warnings'); case self::ISSUE_SUBFAIL: @@ -145,9 +148,11 @@ case self::ISSUE_NULLABLE: return pht('This schema has the wrong nullable setting.'); case self::ISSUE_KEYCOLUMNS: - return pht('This schema is on the wrong columns.'); + return pht('This key is on the wrong columns.'); case self::ISSUE_UNIQUE: return pht('This key has the wrong uniqueness setting.'); + case self::ISSUE_LONGKEY: + return pht('This key is too long for utf8mb4.'); case self::ISSUE_SUBWARN: return pht('Subschemata have setup warnings.'); case self::ISSUE_SUBFAIL: @@ -172,6 +177,7 @@ case self::ISSUE_SURPLUSKEY: case self::ISSUE_UNIQUE: case self::ISSUE_KEYCOLUMNS: + case self::ISSUE_LONGKEY: return self::STATUS_WARN; default: throw new Exception(pht('Unknown schema issue "%s"!', $issue)); diff --git a/src/applications/config/schema/PhabricatorConfigTableSchema.php b/src/applications/config/schema/PhabricatorConfigTableSchema.php --- a/src/applications/config/schema/PhabricatorConfigTableSchema.php +++ b/src/applications/config/schema/PhabricatorConfigTableSchema.php @@ -23,6 +23,7 @@ throw new Exception( pht('Trying to add duplicate key "%s"!', $name)); } + $key->setTable($this); $this->keys[$name] = $key; return $this; } @@ -44,7 +45,12 @@ } protected function getSubschemata() { - return array_merge($this->getColumns(), $this->getKeys()); + // NOTE: Keys and columns may have the same name, so make sure we return + // everything. + + return array_merge( + array_values($this->columns), + array_values($this->keys)); } public function setCollation($collation) { diff --git a/src/applications/differential/storage/DifferentialDiffProperty.php b/src/applications/differential/storage/DifferentialDiffProperty.php --- a/src/applications/differential/storage/DifferentialDiffProperty.php +++ b/src/applications/differential/storage/DifferentialDiffProperty.php @@ -12,7 +12,7 @@ 'data' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', ), self::CONFIG_KEY_SCHEMA => array( 'diffID' => array( diff --git a/src/applications/diviner/storage/DivinerLiveSymbol.php b/src/applications/diviner/storage/DivinerLiveSymbol.php --- a/src/applications/diviner/storage/DivinerLiveSymbol.php +++ b/src/applications/diviner/storage/DivinerLiveSymbol.php @@ -69,7 +69,7 @@ ), ), 'name' => array( - 'columns' => array('name'), + 'columns' => array('name(64)'), ), 'key_slug' => array( 'columns' => array('titleSlugHash'), diff --git a/src/applications/files/storage/PhabricatorTransformedFile.php b/src/applications/files/storage/PhabricatorTransformedFile.php --- a/src/applications/files/storage/PhabricatorTransformedFile.php +++ b/src/applications/files/storage/PhabricatorTransformedFile.php @@ -9,7 +9,7 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'transform' => 'text255', + 'transform' => 'text128', ), self::CONFIG_KEY_SCHEMA => array( 'originalPHID' => array( diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -322,21 +322,14 @@ $rule->attachActions($actions); if (!$errors) { - try { - - $edit_action = $rule->getID() ? 'edit' : 'create'; - - $rule->openTransaction(); - $rule->save(); - $rule->saveConditions($conditions); - $rule->saveActions($actions); - $rule->logEdit($request->getUser()->getPHID(), $edit_action); - $rule->saveTransaction(); - - } catch (AphrontDuplicateKeyQueryException $ex) { - $e_name = pht('Not Unique'); - $errors[] = pht('Rule name is not unique. Choose a unique name.'); - } + $edit_action = $rule->getID() ? 'edit' : 'create'; + + $rule->openTransaction(); + $rule->save(); + $rule->saveConditions($conditions); + $rule->saveActions($actions); + $rule->logEdit($request->getUser()->getPHID(), $edit_action); + $rule->saveTransaction(); } return array($e_name, $errors); diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -36,7 +36,7 @@ 'contentType' => 'text255', 'mustMatchAll' => 'bool', 'configVersion' => 'uint32', - 'ruleType' => 'text255', + 'ruleType' => 'text32', 'isDisabled' => 'uint32', 'triggerObjectPHID' => 'phid?', @@ -45,16 +45,10 @@ 'repetitionPolicy' => 'uint32?', ), self::CONFIG_KEY_SCHEMA => array( - 'key_phid' => null, - 'phid' => array( - 'columns' => array('phid'), - 'unique' => true, + 'key_author' => array( + 'columns' => array('authorPHID'), ), - 'authorPHID' => array( - 'columns' => array('authorPHID', 'name'), - 'unique' => true, - ), - 'IDX_RULE_TYPE' => array( + 'key_ruletype' => array( 'columns' => array('ruleType'), ), 'key_trigger' => array( diff --git a/src/applications/herald/storage/transcript/HeraldTranscript.php b/src/applications/herald/storage/transcript/HeraldTranscript.php --- a/src/applications/herald/storage/transcript/HeraldTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldTranscript.php @@ -106,6 +106,7 @@ 'host' => 'text255', 'duration' => 'double', 'dryRun' => 'bool', + 'garbageCollected' => 'bool', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, diff --git a/src/applications/macro/storage/PhabricatorFileImageMacro.php b/src/applications/macro/storage/PhabricatorFileImageMacro.php --- a/src/applications/macro/storage/PhabricatorFileImageMacro.php +++ b/src/applications/macro/storage/PhabricatorFileImageMacro.php @@ -44,7 +44,7 @@ return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'authorPHID' => 'phid?', 'isDisabled' => 'bool', 'audioPHID' => 'phid?', diff --git a/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php b/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php --- a/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php +++ b/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php @@ -18,8 +18,8 @@ return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', - 'email' => 'text255', + 'name' => 'text128', + 'email' => 'text128', 'uri' => 'text255?', ), self::CONFIG_KEY_SCHEMA => array( diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -33,7 +33,7 @@ 'parameters' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'status' => 'text255', + 'status' => 'text32', 'relatedPHID' => 'phid?', // T6203/NULLABILITY diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -39,7 +39,7 @@ self::CONFIG_TIMESTAMPS => false, self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'originalName' => 'text255', 'description' => 'text', 'primaryOwnerPHID' => 'phid?', diff --git a/src/applications/people/storage/PhabricatorExternalAccount.php b/src/applications/people/storage/PhabricatorExternalAccount.php --- a/src/applications/people/storage/PhabricatorExternalAccount.php +++ b/src/applications/people/storage/PhabricatorExternalAccount.php @@ -44,7 +44,7 @@ 'accountType' => 'text16', 'accountDomain' => 'text64', 'accountSecret' => 'text?', - 'accountID' => 'text160', + 'accountID' => 'text64', 'displayName' => 'text255?', 'username' => 'text255?', 'realName' => 'text255?', diff --git a/src/applications/people/storage/PhabricatorUserSchemaSpec.php b/src/applications/people/storage/PhabricatorUserSchemaSpec.php --- a/src/applications/people/storage/PhabricatorUserSchemaSpec.php +++ b/src/applications/people/storage/PhabricatorUserSchemaSpec.php @@ -26,7 +26,7 @@ ), array( 'token' => array( - 'columns' => array('token'), + 'columns' => array('token(128)'), ), 'userID' => array( 'columns' => array('userID'), diff --git a/src/applications/phame/storage/PhameBlog.php b/src/applications/phame/storage/PhameBlog.php --- a/src/applications/phame/storage/PhameBlog.php +++ b/src/applications/phame/storage/PhameBlog.php @@ -30,7 +30,7 @@ self::CONFIG_COLUMN_SCHEMA => array( 'name' => 'text64', 'description' => 'text', - 'domain' => 'text255?', + 'domain' => 'text128?', // T6203/NULLABILITY // These policies should always be non-null. diff --git a/src/applications/phragment/storage/PhragmentFragment.php b/src/applications/phragment/storage/PhragmentFragment.php --- a/src/applications/phragment/storage/PhragmentFragment.php +++ b/src/applications/phragment/storage/PhragmentFragment.php @@ -15,7 +15,7 @@ return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'path' => 'text255', + 'path' => 'text128', 'depth' => 'uint32', 'latestVersionPHID' => 'phid?', ), diff --git a/src/applications/phragment/storage/PhragmentSnapshot.php b/src/applications/phragment/storage/PhragmentSnapshot.php --- a/src/applications/phragment/storage/PhragmentSnapshot.php +++ b/src/applications/phragment/storage/PhragmentSnapshot.php @@ -12,7 +12,7 @@ return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', ), self::CONFIG_KEY_SCHEMA => array( 'key_name' => array( diff --git a/src/applications/phriction/storage/PhrictionContent.php b/src/applications/phriction/storage/PhrictionContent.php --- a/src/applications/phriction/storage/PhrictionContent.php +++ b/src/applications/phriction/storage/PhrictionContent.php @@ -51,7 +51,7 @@ 'columns' => array('authorPHID'), ), 'slug' => array( - 'columns' => array('slug(255)'), + 'columns' => array('slug'), ), ), ) + parent::getConfiguration(); diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -122,7 +122,7 @@ 'subprojectPHIDs' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'status' => 'text32', 'phrictionSlug' => 'text128?', 'isMembershipLocked' => 'bool', diff --git a/src/applications/releeph/storage/ReleephProject.php b/src/applications/releeph/storage/ReleephProject.php --- a/src/applications/releeph/storage/ReleephProject.php +++ b/src/applications/releeph/storage/ReleephProject.php @@ -30,7 +30,7 @@ 'details' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'trunkBranch' => 'text255', 'isActive' => 'bool', ), diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -97,7 +97,7 @@ 'unique' => true, ), 'key_name' => array( - 'columns' => array('name'), + 'columns' => array('name(128)'), ), 'key_vcs' => array( 'columns' => array('versionControlSystem'), diff --git a/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php b/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php --- a/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php +++ b/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php @@ -23,7 +23,7 @@ 'symbolIndexProjects' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'repositoryID' => 'id?', ), self::CONFIG_KEY_SCHEMA => array( diff --git a/src/applications/repository/storage/PhabricatorRepositoryBranch.php b/src/applications/repository/storage/PhabricatorRepositoryBranch.php --- a/src/applications/repository/storage/PhabricatorRepositoryBranch.php +++ b/src/applications/repository/storage/PhabricatorRepositoryBranch.php @@ -9,7 +9,7 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'text128', 'lintCommit' => 'text40?', ), self::CONFIG_KEY_SCHEMA => array( diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php --- a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php @@ -20,7 +20,7 @@ 'commitDetails' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'authorName' => 'text255', + 'authorName' => 'text', 'commitMessage' => 'text', ), self::CONFIG_KEY_SCHEMA => array( @@ -28,9 +28,6 @@ 'columns' => array('commitID'), 'unique' => true, ), - 'authorName' => array( - 'columns' => array('authorName'), - ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php --- a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php +++ b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php @@ -15,7 +15,7 @@ id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_BADCOMMIT, array( - 'fullCommitName' => 'text255', + 'fullCommitName' => 'text64', 'description' => 'text', ), array( diff --git a/src/applications/search/storage/document/PhabricatorSearchDocumentField.php b/src/applications/search/storage/document/PhabricatorSearchDocumentField.php --- a/src/applications/search/storage/document/PhabricatorSearchDocumentField.php +++ b/src/applications/search/storage/document/PhabricatorSearchDocumentField.php @@ -22,10 +22,9 @@ 'phid' => array( 'columns' => array('phid'), ), - - // NOTE: This is a fulltext index! Be careful! 'corpus' => array( 'columns' => array('corpus'), + 'type' => 'FULLTEXT', ), ), ) + parent::getConfiguration(); diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php @@ -17,8 +17,8 @@ public function getConfiguration() { return array( self::CONFIG_COLUMN_SCHEMA => array( - 'taskClass' => 'text255', - 'leaseOwner' => 'text255?', + 'taskClass' => 'text64', + 'leaseOwner' => 'text64?', 'leaseExpires' => 'epoch?', 'failureCount' => 'uint32', 'failureTime' => 'epoch?', diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php @@ -14,8 +14,10 @@ } public function execute(PhutilArgumentParser $args) { + $force = $args->getArg('force'); + $this->requireAllPatchesApplied(); - $this->adjustSchemata(); + $this->adjustSchemata($force); return 0; } @@ -58,7 +60,7 @@ return array($comp, $expect, $actual); } - private function adjustSchemata() { + private function adjustSchemata($force) { $console = PhutilConsole::getConsole(); $console->writeOut( @@ -98,26 +100,28 @@ $table->draw(); - $console->writeOut( - "\n%s\n", - pht( - "Found %s issues(s) with schemata, detailed above.\n\n". - "You can review issues in more detail from the web interface, ". - "in Config > Database Status.\n\n". - "MySQL needs to copy table data to make some adjustments, so these ". - "migrations may take some time.". - - // TODO: Remove warning once this stabilizes. - "\n\n". - "WARNING: This workflow is new and unstable. If you continue, you ". - "may unrecoverably destory data. Make sure you have a backup before ". - "you proceed.", - - new PhutilNumber(count($adjustments)))); - - $prompt = pht('Fix these schema issues?'); - if (!phutil_console_confirm($prompt, $default_no = true)) { - return; + if (!$force) { + $console->writeOut( + "\n%s\n", + pht( + "Found %s issues(s) with schemata, detailed above.\n\n". + "You can review issues in more detail from the web interface, ". + "in Config > Database Status.\n\n". + "MySQL needs to copy table data to make some adjustments, so these ". + "migrations may take some time.". + + // TODO: Remove warning once this stabilizes. + "\n\n". + "WARNING: This workflow is new and unstable. If you continue, you ". + "may unrecoverably destory data. Make sure you have a backup before ". + "you proceed.", + + new PhutilNumber(count($adjustments)))); + + $prompt = pht('Fix these schema issues?'); + if (!phutil_console_confirm($prompt, $default_no = true)) { + return; + } } $console->writeOut( @@ -194,22 +198,47 @@ break; case 'key': if (($phase == 0) && $adjust['exists']) { + if ($adjust['name'] == 'PRIMARY') { + $key_name = 'PRIMARY KEY'; + } else { + $key_name = qsprintf($conn, 'KEY %T', $adjust['name']); + } + queryfx( $conn, - 'ALTER TABLE %T.%T DROP KEY %T', + 'ALTER TABLE %T.%T DROP %Q', $adjust['database'], $adjust['table'], - $adjust['name']); + $key_name); } if (($phase == 2) && $adjust['keep']) { + // Different keys need different creation syntax. Notable + // special cases are primary keys and fulltext keys. + if ($adjust['name'] == 'PRIMARY') { + $key_name = 'PRIMARY KEY'; + } else if ($adjust['indexType'] == 'FULLTEXT') { + $key_name = qsprintf($conn, 'FULLTEXT %T', $adjust['name']); + } else { + if ($adjust['unique']) { + $key_name = qsprintf( + $conn, + 'UNIQUE KEY %T', + $adjust['name']); + } else { + $key_name = qsprintf( + $conn, + '/* NONUNIQUE */ KEY %T', + $adjust['name']); + } + } + queryfx( $conn, - 'ALTER TABLE %T.%T ADD %Q KEY %T (%Q)', + 'ALTER TABLE %T.%T ADD %Q (%Q)', $adjust['database'], $adjust['table'], - $adjust['unique'] ? 'UNIQUE' : '/* NONUNIQUE */', - $adjust['name'], + $key_name, implode(', ', $adjust['columns'])); } break; @@ -239,7 +268,7 @@ foreach ($failed as $failure) { list($adjust, $ex) = $failure; - $pieces = array_select_keys($adjust, array('database', 'table', 'naeme')); + $pieces = array_select_keys($adjust, array('database', 'table', 'name')); $pieces = array_filter($pieces); $target = implode('.', $pieces); @@ -269,7 +298,7 @@ $issue_missingkey = PhabricatorConfigStorageSchema::ISSUE_MISSINGKEY; $issue_columns = PhabricatorConfigStorageSchema::ISSUE_KEYCOLUMNS; $issue_unique = PhabricatorConfigStorageSchema::ISSUE_UNIQUE; - + $issue_longkey = PhabricatorConfigStorageSchema::ISSUE_LONGKEY; $adjustments = array(); foreach ($comp->getDatabases() as $database_name => $database) { @@ -393,6 +422,16 @@ $issues[] = $issue_unique; } + // NOTE: We can't really fix this, per se, but we may need to remove + // the key to change the column type. In the best case, the new + // column type won't be overlong and recreating the key really will + // fix the issue. In the worst case, we get the right column type and + // lose the key, which is still better than retaining the key having + // the wrong column type. + if ($key->hasIssue($issue_longkey)) { + $issues[] = $issue_longkey; + } + if ($issues) { $adjustment = array( 'kind' => 'key', @@ -408,6 +447,7 @@ $adjustment += array( 'columns' => $expect_key->getColumnNames(), 'unique' => $expect_key->getUnique(), + 'indexType' => $expect_key->getIndexType(), ); } diff --git a/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php b/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php --- a/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php +++ b/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php @@ -8,7 +8,7 @@ 'meta_data', 'patch_status', array( - 'patch' => 'text255', + 'patch' => 'text128', 'applied' => 'uint32', ), array(