1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-19 05:12:41 +01:00

Fix almost all remaining schemata issues

Summary:
Ref T1191. This fixes nearly every remaining blocker for utf8mb4 -- primarily, overlong keys.

Remaining issue is https://secure.phabricator.com/T1191#77467

Test Plan: I'll annotate inline.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, hach-que

Maniphest Tasks: T6099, T6129, T6133, T6134, T6150, T6148, T6147, T6146, T6105, T1191

Differential Revision: https://secure.phabricator.com/D10601
This commit is contained in:
epriestley 2014-10-01 08:18:36 -07:00
parent a5ce56aa76
commit 4fcc634a99
37 changed files with 181 additions and 90 deletions

View file

@ -32,7 +32,7 @@ final class PhabricatorAuthProviderConfig extends PhabricatorAuthDAO
self::CONFIG_COLUMN_SCHEMA => array(
'isEnabled' => 'bool',
'providerClass' => 'text128',
'providerType' => 'text64',
'providerType' => 'text32',
'providerDomain' => 'text128',
'shouldAllowLogin' => 'bool',
'shouldAllowRegistration' => 'bool',

View file

@ -15,7 +15,7 @@ final class PhabricatorConduitMethodCallLog
self::CONFIG_COLUMN_SCHEMA => array(
'id' => 'id64',
'connectionID' => 'id64?',
'method' => 'text255',
'method' => 'text64',
'error' => 'text255',
'duration' => 'uint64',
'callerPHID' => 'phid?',

View file

@ -3,8 +3,6 @@
abstract class PhabricatorConfigDatabaseController
extends PhabricatorConfigController {
const MAX_INNODB_KEY_LENGTH = 767;
protected function buildSchemaQuery() {
$conf = PhabricatorEnv::newObjectFromConfig(
'mysql.configuration-provider',

View file

@ -281,6 +281,7 @@ final class PhabricatorConfigDatabaseStatusController
$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 @@ final class PhabricatorConfigDatabaseStatusController
if ($size) {
$size_formatted = $this->renderAttr(
$size,
($size > self::MAX_INNODB_KEY_LENGTH));
$key->hasIssue($longkey_issue));
}
$key_rows[] = array(

View file

@ -62,7 +62,7 @@ final class PhabricatorConfigColumnSchema
$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 @@ final class PhabricatorConfigColumnSchema
}
$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) {

View file

@ -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 @@ final class PhabricatorConfigKeySchema
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 @@ final class PhabricatorConfigKeySchema
}
}
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 @@ final class PhabricatorConfigKeySchema
$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;
}

View file

@ -129,7 +129,8 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
$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);
}

View file

@ -114,6 +114,7 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
->setColumnNames(idx($key_spec, 'columns', array()));
$key->setUnique((bool)idx($key_spec, 'unique'));
$key->setIndexType(idx($key_spec, 'type', 'BTREE'));
$table->addKey($key);
}

View file

@ -12,6 +12,7 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
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 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
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));

View file

@ -23,6 +23,7 @@ final class PhabricatorConfigTableSchema
throw new Exception(
pht('Trying to add duplicate key "%s"!', $name));
}
$key->setTable($this);
$this->keys[$name] = $key;
return $this;
}
@ -44,7 +45,12 @@ final class PhabricatorConfigTableSchema
}
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) {

View file

@ -12,7 +12,7 @@ final class DifferentialDiffProperty extends DifferentialDAO {
'data' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
),
self::CONFIG_KEY_SCHEMA => array(
'diffID' => array(

View file

@ -69,7 +69,7 @@ final class DivinerLiveSymbol extends DivinerDAO
),
),
'name' => array(
'columns' => array('name'),
'columns' => array('name(64)'),
),
'key_slug' => array(
'columns' => array('titleSlugHash'),

View file

@ -9,7 +9,7 @@ final class PhabricatorTransformedFile extends PhabricatorFileDAO {
public function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
'transform' => 'text255',
'transform' => 'text128',
),
self::CONFIG_KEY_SCHEMA => array(
'originalPHID' => array(

View file

@ -322,21 +322,14 @@ final class HeraldRuleController extends HeraldController {
$rule->attachActions($actions);
if (!$errors) {
try {
$edit_action = $rule->getID() ? 'edit' : 'create';
$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.');
}
$rule->openTransaction();
$rule->save();
$rule->saveConditions($conditions);
$rule->saveActions($actions);
$rule->logEdit($request->getUser()->getPHID(), $edit_action);
$rule->saveTransaction();
}
return array($e_name, $errors);

View file

@ -36,7 +36,7 @@ final class HeraldRule extends HeraldDAO
'contentType' => 'text255',
'mustMatchAll' => 'bool',
'configVersion' => 'uint32',
'ruleType' => 'text255',
'ruleType' => 'text32',
'isDisabled' => 'uint32',
'triggerObjectPHID' => 'phid?',
@ -45,16 +45,10 @@ final class HeraldRule extends HeraldDAO
'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(

View file

@ -106,6 +106,7 @@ final class HeraldTranscript extends HeraldDAO
'host' => 'text255',
'duration' => 'double',
'dryRun' => 'bool',
'garbageCollected' => 'bool',
),
self::CONFIG_KEY_SCHEMA => array(
'key_phid' => null,

View file

@ -44,7 +44,7 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
return array(
self::CONFIG_AUX_PHID => true,
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'authorPHID' => 'phid?',
'isDisabled' => 'bool',
'audioPHID' => 'phid?',

View file

@ -18,8 +18,8 @@ final class PhabricatorMetaMTAMailingList extends PhabricatorMetaMTADAO
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(

View file

@ -33,7 +33,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
'parameters' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'status' => 'text255',
'status' => 'text32',
'relatedPHID' => 'phid?',
// T6203/NULLABILITY

View file

@ -39,7 +39,7 @@ final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO
self::CONFIG_TIMESTAMPS => false,
self::CONFIG_AUX_PHID => true,
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'originalName' => 'text255',
'description' => 'text',
'primaryOwnerPHID' => 'phid?',

View file

@ -44,7 +44,7 @@ final class PhabricatorExternalAccount extends PhabricatorUserDAO
'accountType' => 'text16',
'accountDomain' => 'text64',
'accountSecret' => 'text?',
'accountID' => 'text160',
'accountID' => 'text64',
'displayName' => 'text255?',
'username' => 'text255?',
'realName' => 'text255?',

View file

@ -26,7 +26,7 @@ final class PhabricatorUserSchemaSpec extends PhabricatorConfigSchemaSpec {
),
array(
'token' => array(
'columns' => array('token'),
'columns' => array('token(128)'),
),
'userID' => array(
'columns' => array('userID'),

View file

@ -30,7 +30,7 @@ final class PhameBlog extends PhameDAO
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text64',
'description' => 'text',
'domain' => 'text255?',
'domain' => 'text128?',
// T6203/NULLABILITY
// These policies should always be non-null.

View file

@ -15,7 +15,7 @@ final class PhragmentFragment extends PhragmentDAO
return array(
self::CONFIG_AUX_PHID => true,
self::CONFIG_COLUMN_SCHEMA => array(
'path' => 'text255',
'path' => 'text128',
'depth' => 'uint32',
'latestVersionPHID' => 'phid?',
),

View file

@ -12,7 +12,7 @@ final class PhragmentSnapshot extends PhragmentDAO
return array(
self::CONFIG_AUX_PHID => true,
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
),
self::CONFIG_KEY_SCHEMA => array(
'key_name' => array(

View file

@ -51,7 +51,7 @@ final class PhrictionContent extends PhrictionDAO
'columns' => array('authorPHID'),
),
'slug' => array(
'columns' => array('slug(255)'),
'columns' => array('slug'),
),
),
) + parent::getConfiguration();

View file

@ -122,7 +122,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO
'subprojectPHIDs' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'status' => 'text32',
'phrictionSlug' => 'text128?',
'isMembershipLocked' => 'bool',

View file

@ -30,7 +30,7 @@ final class ReleephProject extends ReleephDAO
'details' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'trunkBranch' => 'text255',
'isActive' => 'bool',
),

View file

@ -97,7 +97,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
'unique' => true,
),
'key_name' => array(
'columns' => array('name'),
'columns' => array('name(128)'),
),
'key_vcs' => array(
'columns' => array('versionControlSystem'),

View file

@ -23,7 +23,7 @@ final class PhabricatorRepositoryArcanistProject
'symbolIndexProjects' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'repositoryID' => 'id?',
),
self::CONFIG_KEY_SCHEMA => array(

View file

@ -9,7 +9,7 @@ final class PhabricatorRepositoryBranch extends PhabricatorRepositoryDAO {
public function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'name' => 'text128',
'lintCommit' => 'text40?',
),
self::CONFIG_KEY_SCHEMA => array(

View file

@ -20,7 +20,7 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
'commitDetails' => self::SERIALIZATION_JSON,
),
self::CONFIG_COLUMN_SCHEMA => array(
'authorName' => 'text255',
'authorName' => 'text',
'commitMessage' => 'text',
),
self::CONFIG_KEY_SCHEMA => array(
@ -28,9 +28,6 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
'columns' => array('commitID'),
'unique' => true,
),
'authorName' => array(
'columns' => array('authorName'),
),
),
) + parent::getConfiguration();
}

View file

@ -15,7 +15,7 @@ final class PhabricatorRepositorySchemaSpec
id(new PhabricatorRepository())->getApplicationName(),
PhabricatorRepository::TABLE_BADCOMMIT,
array(
'fullCommitName' => 'text255',
'fullCommitName' => 'text64',
'description' => 'text',
),
array(

View file

@ -22,10 +22,9 @@ final class PhabricatorSearchDocumentField extends PhabricatorSearchDAO {
'phid' => array(
'columns' => array('phid'),
),
// NOTE: This is a fulltext index! Be careful!
'corpus' => array(
'columns' => array('corpus'),
'type' => 'FULLTEXT',
),
),
) + parent::getConfiguration();

View file

@ -17,8 +17,8 @@ abstract class PhabricatorWorkerTask extends PhabricatorWorkerDAO {
public function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
'taskClass' => 'text255',
'leaseOwner' => 'text255?',
'taskClass' => 'text64',
'leaseOwner' => 'text64?',
'leaseExpires' => 'epoch?',
'failureCount' => 'uint32',
'failureTime' => 'epoch?',

View file

@ -14,8 +14,10 @@ final class PhabricatorStorageManagementAdjustWorkflow
}
public function execute(PhutilArgumentParser $args) {
$force = $args->getArg('force');
$this->requireAllPatchesApplied();
$this->adjustSchemata();
$this->adjustSchemata($force);
return 0;
}
@ -58,7 +60,7 @@ final class PhabricatorStorageManagementAdjustWorkflow
return array($comp, $expect, $actual);
}
private function adjustSchemata() {
private function adjustSchemata($force) {
$console = PhutilConsole::getConsole();
$console->writeOut(
@ -98,26 +100,28 @@ final class PhabricatorStorageManagementAdjustWorkflow
$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.".
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.",
// 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))));
new PhutilNumber(count($adjustments))));
$prompt = pht('Fix these schema issues?');
if (!phutil_console_confirm($prompt, $default_no = true)) {
return;
$prompt = pht('Fix these schema issues?');
if (!phutil_console_confirm($prompt, $default_no = true)) {
return;
}
}
$console->writeOut(
@ -194,22 +198,47 @@ final class PhabricatorStorageManagementAdjustWorkflow
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 @@ final class PhabricatorStorageManagementAdjustWorkflow
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 @@ final class PhabricatorStorageManagementAdjustWorkflow
$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 @@ final class PhabricatorStorageManagementAdjustWorkflow
$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 @@ final class PhabricatorStorageManagementAdjustWorkflow
$adjustment += array(
'columns' => $expect_key->getColumnNames(),
'unique' => $expect_key->getUnique(),
'indexType' => $expect_key->getIndexType(),
);
}

View file

@ -8,7 +8,7 @@ final class PhabricatorStorageSchemaSpec
'meta_data',
'patch_status',
array(
'patch' => 'text255',
'patch' => 'text128',
'applied' => 'uint32',
),
array(